-
-
Notifications
You must be signed in to change notification settings - Fork 62
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for Elusive Target Arcade #209
Conversation
This comment was marked as duplicate.
This comment was marked as duplicate.
We're not going to support the 12 hour lockout, but I'm not sure if we should reset it either on fail. I've put it to a poll and it will conclude in 24-48 hours. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good!
* when filtering out challenges for the parent location, but it is | ||
* much more efficient to do it this way. | ||
*/ | ||
export const parentsWithETA = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My fingers are itching by this bit and locationsWithETA
, seems like premature optimization.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll defer this to @moonysolari.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both of these arrays are used solely in the filterChallenge
function. When getting the challenges for a contract or a location, this function is called on every challenge that is either in the parent location or a global challenge.
To do without locationsWithETA
, we will need to loop over all arcade levels and run resolveContract
to get their locations every time this array is needed.
To do without parentsWithETA
, we will need to loop over all locations in locationsWithETA
and get their parent locations from the config file every time this array is needed.
I wrote them as constants considering the frequency of filterChallenge
being called, the complexity of acquiring the locations with ETA in real-time, and the likelihood that this list will change. However, this implementation is still up for discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But why not build it once at runtime, instead of hardcoding it? This is going to give issues if someone is going to add custom stuff through plugins.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Addressed as required.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You also need to remove the arrays from elusiveTargetArcades.ts
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - do we want to get rid of the whole orderedET array as well and just built that runtime too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't, there's no ordering data for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In that case I sign off on this in its current state, good job all :)
This PR adds support for Elusive Target Arcade.
What's left to do/fix (feel free to add more):
Incorrect challenges
Currently, we use the group contract's location ID for challenges/mastery, which is fine for the main gamemode page, but not for the contracts as they span different locations so this means the planning page has incorrect challenges for the contract location, as well as in-game challenges (i.e. the ones being tracked), and the final mission end challenges/mastery.
IOI also at some point changed from setting the group location to the first level's location to the final level's location (which being honest, makes more sense).