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

refactor!: game versioned missionsInLocation, fix certain contracts when resolved, rename missionsInLocations -> missionsInLocation #550

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

AnthonyFuller
Copy link
Contributor

@AnthonyFuller AnthonyFuller commented Jan 4, 2025

Fixes #510 #505 #504

Needs testing and checking over more than twice.

Breaking Changes

  • controller.missionsInLocations is now controller.missionsInLocation (as it always should have been!).
  • The aforementioned controller.missionsInLocations is now split by game version.
  • controller.addEscalation has a new parameter: (groupContract, locationId, **gameVersion**, ...levels) (highlighted using asterisks).

Also this removes some terrible hacks we've had to do. We should've always done it like this!

@AnthonyFuller AnthonyFuller requested a review from RDIL January 4, 2025 01:40
@AnthonyFuller
Copy link
Contributor Author

Discussion question before it should be considered to be merged: should controller.addEscalation even add it to missionsInLocation or should that be up to the plugin? (scanForGroups may need to be made public if we leave it up to the plugin)

Copy link
Member

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

See above

@RDIL
Copy link
Member

RDIL commented Jan 4, 2025

I think we have a scan for groups hook?

@AnthonyFuller
Copy link
Contributor Author

I think we have a scan for groups hook?

We do, but it is named getContractIdsForGroupDiscovery. Is it worth moving over to this?

@AnthonyFuller AnthonyFuller requested a review from RDIL January 4, 2025 19:37
"4fbfae2e-a5e7-4b79-b008-94f6cbcb13cb",
"3721e543-b5e6-4af8-a4fc-c92e9a4453bd",
"8c6daf5e-5974-4438-af20-71ff570c7ff3",
export const missionsInLocation = {
Copy link
Member

Choose a reason for hiding this comment

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

Explicitly type this object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this not satisfy that?

components/controller.ts Outdated Show resolved Hide resolved
Copy link
Member

@RDIL RDIL left a comment

Choose a reason for hiding this comment

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

LGTM % ^

@AnthonyFuller
Copy link
Contributor Author

AnthonyFuller commented Jan 5, 2025

Sorry @ Private for that tag. Didn't even expect that.


What about controller.addEscalation?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants