Skip to content
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

Merged
merged 39 commits into from
Apr 21, 2023
Merged

Add support for Elusive Target Arcade #209

merged 39 commits into from
Apr 21, 2023

Conversation

AnthonyFuller
Copy link
Contributor

@AnthonyFuller AnthonyFuller commented Apr 14, 2023

This PR adds support for Elusive Target Arcade.

What's left to do/fix (feel free to add more):

  • Incorrect challenges (elaborated below)
  • Arcade challenges in general
  • Failing at one of the later ETs in the series does not reset you back to the first ET.
  • Global arcade challenges added, but not working yet.
  • Arcade challenges don't show in the Career - Challenges page.
  • Arcade challenges are not correctly filtered in the Destination - Challenges tab. (Correct behavior: if the location has an ET, then all global arcade challenges are shown. Additionally, show challenges of arcade contracts assigned to this location.)

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).

@moonysolari moonysolari linked an issue Apr 14, 2023 that may be closed by this pull request
1 task
@moonysolari

This comment was marked as duplicate.

@AnthonyFuller
Copy link
Contributor Author

Issue: failing at one of the later ETs in the series does not reset you back to the first ET.

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.

@moonysolari moonysolari marked this pull request as ready for review April 17, 2023 03:43
@moonysolari moonysolari linked an issue Apr 17, 2023 that may be closed by this pull request
5 tasks
Copy link
Member

@LennardF1989 LennardF1989 left a 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 = [
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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?

Copy link
Contributor Author

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.

Copy link
Member

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 :)

@AnthonyFuller AnthonyFuller merged commit c2d477f into v6 Apr 21, 2023
@AnthonyFuller AnthonyFuller deleted the arcade branch April 23, 2023 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support profile-scoped challenges Support ET arcade
4 participants