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

Rewrite the escalation service to use group contracts #63

Merged
merged 58 commits into from
Apr 14, 2023
Merged

Conversation

RDIL
Copy link
Member

@RDIL RDIL commented Dec 15, 2022

Now squashed!

@RDIL RDIL added major This will need to be in a semver-major release. escalations Related to the escalation system or a specific escalation. labels Dec 15, 2022
@RDIL RDIL added this to the v6 milestone Dec 15, 2022
@RDIL RDIL added urgent This needs to be prioritized. and removed urgent This needs to be prioritized. labels Feb 2, 2023
@RDIL RDIL modified the milestones: v6, v7 Feb 5, 2023
@RDIL RDIL changed the base branch from v6 to codeql March 23, 2023 13:49
@RDIL RDIL changed the base branch from codeql to v6 March 23, 2023 13:49
@AnthonyFuller
Copy link
Contributor

What's left to be done with this?

@moonysolari
Copy link
Member

Waiting for Kaki to get all the latest escalation group definitions and level definitions from official.

@VolkerSchlegel
Copy link
Member

Most escalations are done, our custom escalations still don't have group definitions.

Copy link
Contributor

@AnthonyFuller AnthonyFuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One little snag, but other than that LGTM. Not checked any of the contracts we've added/moved as from experience in playing a few, they're all good.

Copy link
Member

@moonysolari moonysolari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. The escalation level picker needs to be fixed/updated afterwards. Doesn't have to be part of this PR though.

@moonysolari
Copy link
Member

Also we need to make sure whether we want to include legacy escalations or not.

Theoretically they should always be included in H1, but not in H2/H3. Therefore I think we can include them in the code but only show them for H1, and toggle their appearance on H2/H3 with a flag. Choosing to do this will also remove the need for the legacy escalations plugin.

@AnthonyFuller
Copy link
Contributor

AnthonyFuller commented Apr 12, 2023

Just fixed the escalation level picker. Was a simple fix as I just had to change how it accessed the escalation mappings and got the level count.

RE: Legacy Escalations
If we're including them in core Peacock, there's no reason to keep them behind a flag for H2/3 as they will work on those games without any extra intervention, and if we're going to add them to H2/3, we'll have to add the challenges too just to be complete.

EDIT: In addition to this, including them into core Peacock will increase the file size, and then hiding them behind a flag just seems pointless as people may never play 2016 or enable them.

Copy link
Contributor

@AnthonyFuller AnthonyFuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes just to stop merging before Escalation Codenames has been updated with all the new H3 escalations we've added.

Copy link
Contributor

@AnthonyFuller AnthonyFuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the new H3 escalations to Escalation Codenames. Should be good to go now.

EDIT: Also fixed some issues with escalation completion. What we're going to do with Legacy Escalations still needs to be decided.

@moonysolari
Copy link
Member

What we're going to do with Legacy Escalations still needs to be decided.

I agree that they can be added to all game versions.

@AnthonyFuller AnthonyFuller removed the request for review from LennardF1989 April 13, 2023 21:24
@AnthonyFuller
Copy link
Contributor

What I just pushed makes the hits category for elusives respect the new Season tag, this now removes The Deceivers from the page when playing in Hitman 2016.

@AnthonyFuller AnthonyFuller merged commit 4575924 into v6 Apr 14, 2023
@AnthonyFuller AnthonyFuller deleted the squash-temp branch April 14, 2023 02:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
escalations Related to the escalation system or a specific escalation. major This will need to be in a semver-major release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Escalation challenges are not hooked to the corresponding escalations
5 participants