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 state specific things into new GameState structure. #21193

Open
86 of 87 tasks
ZehMatt opened this issue Jan 13, 2024 · 30 comments
Open
86 of 87 tasks

Refactor game state specific things into new GameState structure. #21193

ZehMatt opened this issue Jan 13, 2024 · 30 comments
Labels
refactor A task that will improve code readability, without changing outcome.

Comments

@ZehMatt
Copy link
Member

ZehMatt commented Jan 13, 2024

Starting with #21122 I created a new struct called GameState_t which should carry all game state related variables. Following variables should be put in GameState_t:

I might have missed a few, but that should give one a good idea what needs to be moved.

  • Once all the variables moved into the GameState the Import code should be changed to load the data first into a temporary GameState and on successful load it can be simply swapped/moved to the global one allowing background loading/saving also.
ZehMatt added a commit to ZehMatt/OpenRCT2 that referenced this issue Jan 20, 2024
ZehMatt added a commit that referenced this issue Jan 20, 2024
#21193: Move gCash to GameState_t, refactor uses
ZehMatt added a commit to ZehMatt/OpenRCT2 that referenced this issue Jan 20, 2024
ZehMatt added a commit that referenced this issue Jan 20, 2024
#21193: Move gInitialCash to GameState_t, refactor uses
@Gymnasiast
Copy link
Member

I’d like to “reserve” these:

  • gGuestInitialCash
  • gGuestInitialHappiness
  • gGuestInitialHunger
  • gGuestInitialThirst

@Harry-Hopkinson
Copy link
Contributor

I could do all of the climate ones to help out.

  • gClimate
  • gClimateCurrent
  • gClimateNext
  • gClimateUpdateTimer

Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue Jan 22, 2024
Also changed a few instances where GetGameState was called inside the same function.
The change in Peep.cpp is needed because of a function conflict. FormatStringID exists both in the global and in the OpenRCT2 namespace.
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue Jan 22, 2024
Also changed a few instances where GetGameState was called inside the same function.
The change in Peep.cpp is needed because of a function conflict. FormatStringID exists both in the global and in the OpenRCT2 namespace.
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue Jan 22, 2024
Also changed a few instances where GetGameState was called inside the same function.
The change in Peep.cpp is needed because of a function conflict. FormatStringID exists both in the global and in the OpenRCT2 namespace.
@Gymnasiast Gymnasiast pinned this issue Jan 22, 2024
@Gymnasiast Gymnasiast added the refactor A task that will improve code readability, without changing outcome. label Jan 22, 2024
@Gymnasiast
Copy link
Member

I’d like to reserve:

  • gWeeklyProfitAverageDividend
  • gWeeklyProfitAverageDivisor
  • gWeeklyProfitHistory

@Broxzier
Copy link
Member

I'd like to reserve the remaining gPark... globals:

  • gParkEntranceFee
  • gParkEntrances
  • gParkRatingCasualtyPenalty
  • gParkSize
  • gParkValue
  • gParkValueHistory

Broxzier added a commit that referenced this issue Jan 24, 2024
Broxzier added a commit to Broxzier/OpenRCT2 that referenced this issue Jan 24, 2024
@Gymnasiast
Copy link
Member

I’d like to reserve the gScenario* globals.

@Harry-Hopkinson
Copy link
Contributor

I would like to reserve the following guest related globals...

  • gNextGuestNumber
  • gNumGuestsHeadingForPark
  • gNumGuestsInPark
  • gNumGuestsInParkLastWeek

@jan-kelemen
Copy link
Contributor

I'll do

  • gRideRatingUpdateStates
  • gSamePriceThroughoutPark

@Harry-Hopkinson
Copy link
Contributor

I can do:

  • gLandPrice
  • gLastEntranceStyle

@Gymnasiast
Copy link
Member

@ZehMatt Shouldn’t the following variables be in here as well?

gEntityLists
_freeIdList
_entityFlashingList
gEntitySpatialIndex

@ZehMatt
Copy link
Member Author

ZehMatt commented Mar 7, 2024

@ZehMatt Shouldn’t the following variables be in here as well?

gEntityLists _freeIdList _entityFlashingList gEntitySpatialIndex

No, they are not saved in the file and are typically rebuilt upon loading it.

@jan-kelemen
Copy link
Contributor

Done the gWidePathTileLoopPosition, I've also noticed that gGrassSceneryTileLoopPosition along side it that is saved in the file, so done that too.

@Gymnasiast
Copy link
Member

@ZehMatt Shouldn’t the following variables be in here as well?
gEntityLists _freeIdList _entityFlashingList gEntitySpatialIndex

No, they are not saved in the file and are typically rebuilt upon loading it.

I know they’re not saved in the file, but they are still gamestate-specific. That is to say, they are specific to the currently loaded game and using the list/index of one park on the other will cause severe problems.

@ZehMatt
Copy link
Member Author

ZehMatt commented Mar 9, 2024

@ZehMatt Shouldn’t the following variables be in here as well?
gEntityLists _freeIdList _entityFlashingList gEntitySpatialIndex

No, they are not saved in the file and are typically rebuilt upon loading it.

I know they’re not saved in the file, but they are still gamestate-specific. That is to say, they are specific to the currently loaded game and using the list/index of one park on the other will cause severe problems.

Yeah, we need to refactor that part a bit better but they don't really belong in the game state, that is more a system specific state. I would like to split those things apart so that the entity spatial index is its own thing and not part of the entity registry, the tiles mapping should be also separated. The best way would be to have system based events, example: Entity registry notifies the Spatial mapping that an entity is removed, moving an entity also notifies the spatial system and so on, we need to better separate responsibilities in general.

@Harry-Hopkinson
Copy link
Contributor

I can do the following:

  • gPeepSpawns
  • gPeepWarningThrottle

@Harry-Hopkinson
Copy link
Contributor

Harry-Hopkinson commented Mar 14, 2024

I can do:

gWidePathTileLoopPosition

awards

@jan-kelemen
Copy link
Contributor

I can do:

  • gWidePathTileLoopPosition

@Harry-Hopkinson there is already a PR for that #21554

@Harry-Hopkinson
Copy link
Contributor

Sorry about that, did not realise.

@Gymnasiast
Copy link
Member

@Harry-Hopkinson You can also do gParkRatingCasualtyPenalty, AFAIC. It is assigned by @Broxzier , but he is probably busy as he never made a PR for it.

@tupaschoal
Copy link
Member

Is that it, have we finished moving them All?

@Gymnasiast
Copy link
Member

Hm, maybe. There is a similarly needed structure called GameState which contains some park-related stuff and the date, which I would assume needs to be absorbed into GameState_t. But I’d like @ZehMatt to confirm that.

@ZehMatt
Copy link
Member Author

ZehMatt commented Mar 22, 2024

Hm, maybe. There is a similarly needed structure called GameState which contains some park-related stuff and the date, which I would assume needs to be absorbed into GameState_t. But I’d like @ZehMatt to confirm that.

Yea, all of the data it currently holds should be moved into GameState_t, minor oversight due to how those classes hide the data.

@KatieZeldaKat
Copy link
Contributor

Was gMarketingCampaigns missed in this issue? I feel like this is also a part of the save file.

@Gymnasiast
Copy link
Member

Yes, that seems appropriate. I updated the initial post.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor A task that will improve code readability, without changing outcome.
Projects
None yet
Development

No branches or pull requests

8 participants