-
Notifications
You must be signed in to change notification settings - Fork 647
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
Multi gamemod support #2541
Multi gamemod support #2541
Conversation
/// Apart from that, each of thesehave corresponding CVARS by default, that set an optional # of this group to spawn. | ||
/// Traditionally, it is 2 cargo depots, 1 trade station, and 8 optional POIs. | ||
/// Dynamically added groups will default to 1 option chosen in that group, using the SpawnChance as a weighted chance | ||
/// for the entire group to spawn on a per-POI basis. |
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.
this reformatting here bulldozes IDE presenting meta data, and goes against the conventions throughout the rest of the code base
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.
Weird this is how its done on some other files, but I think no one fixed it
Ill fix it
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.
yea its following convention because tags in comments like summary
get handled by IDEs to create context-sensitive rich UI and generally helps broader maintainability. Most important for public APIs, so you should probably write new ones similar to others you see in the new POI system since you are now intending that system to be publicly accessable/shared
Note: Right now this code cannot be used since we have an issue with UI on lobby not updating per POI unless you relog. Have to fix it before in another PR. |
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.
Looks good as far as i can see
Right now there is still an issue if you do this:
Something with the UI getting fucked and make it impossible to play when the game mod change and you stay in the lobby. |
I am able to join in both gamemodes just fine, i need to know exact console commands to reproduce them |
Off the top of my head, the issue's this:
At this point, even if you close and reopen the UI, you can't join any of the stations - they're all referencing the old station entities. It's a lifecycle issue in the PickerWindow, the fix should be registering for the LobbyJobsAvailableUpdated event in EnteredTree vs. the constructor: |
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.
Largely seems reasonable, small bug, and might recommend using prototype IDs instead of generic strings.
Pushed the POIPrototype into Content.Server (it's ignored on the client as-is), swapped over to use the prototype ID from a generic string, added the cleanup for the station positions. Looks fine to me. |
About the PR
This PR aim to add the support for Pirate / Adventure / Admin events etc
By splitting the game mod code into few files, and adding support for POI per gamemod.
Why / Balance
Support for admin use for events + forks.
How to test
Open the game to lobby, swap gamemods as you wise.
Media
Requirements
Breaking changes
None, issues fixed
Changelog
Admin change so no note.