TheOpenSpaceProgram / new-ospgl

A space exploration game in OpenGL. Devblog: https://tatjam.github.io/index.html
MIT License
42 stars 6 forks source link

Fix gamestate loading spaghetti #37

Open tatjam opened 1 year ago

tatjam commented 1 year ago

There's a bit of a circular loading dependency when loading a gamestate, where entities require osp->game_state is already set during loading. Fix this by rearranging when the entities create the lua env (assign osp->universe, etc...) so that the load order makes more sense

tigercoding56 commented 1 year ago

something to attempt over summer break , i see (wishful thinking on my part (i wish i could actually , code without 1000th bugs )) -- where is the loading implemented (what file? ) not that i will actually get anywhere (the most complicated thing i ever did in c++ was a remake of flappy bird for intro to CS )

tatjam commented 1 year ago

The loading code right now is a bit of a mess, but you can find the "start of it" here: https://github.com/TheOpenSpaceProgram/new-ospgl/blob/bdb9057c6d802004c71c864320b8c8b09845534f/src/OSP.cpp#L292

If you follow the different calls you can quickly get to the real loading logic, which is mostly on GameState.cpp

A good example of spaghetti is the fact that osp->game_state (which you would expect is set on the launch_gamestate function!) is in fact set by the GameState itself: https://github.com/TheOpenSpaceProgram/new-ospgl/blob/b161469c0943645d43a34da1299a27853cbd1c11/src/game/GameState.cpp#L101

This is kind of required because otherwise as entities and scenes are created, their lua code would run and that code may attempt to access osp->universe, which would not be set as the Universe belongs to the GameState. This circular dependency means that OSP may not have an universe after until the GameState load function has finished (as that creates the universe), but the GameState load may cause some lua code to run which will access osp.universe and crash the game.

The current solution is to set osp.universe within GameState (while the universe is being populated with entities), but a more elegant solution would be to separate the loading logic from any execution of lua code. For example, moving the Entity constructor lua loading code to an external function.