Feature/new game 2#13
Conversation
RomanCantu
commented
Jun 11, 2025
- complete new game features
- still needs to be tested in combination with the UI and the rest of the game
- some adjustments might be necessary to the DefaultInitializer to ensure a fun gameplay
- src/engine/new_game.py could be split into smaller pieces
- TransmissionTopology.connect_n_closest() needs to be reviewed
There was a problem hiding this comment.
It could be nice to use a dataclass instead of a dict for the x/y points
There is a "Point" model already somewhere in the front end that I think should be moved to the core models so you could use that.
The advantage of this is that in many cases the type hint will tell you exactly what to expect from a function and you don't need to explain multiple times that this is a dict with x y points. Also there is some basic geometric arithmetic built in (Like you can linspace points or take the difference between two points etc).
Another TODO I was thinking from the front end is that there should be some notion of the playable map area (I.e the rectangle of coordinates in which everything exists). This could be added to the game state.
This is necessary because when plotting stuff the scale of the objects has to somehow match the screen which will match the map area. Currently for the plotting tests, the playable map area is set up to be -30 to 30 in the X and Y dimensions and the size of the buses is set up to be a good size relative to this area. But ideally the playable map area would come from the game state.
There was a problem hiding this comment.
yeah, we need to have a reference to the size of the map. Would it make sense to have maps with different dimensions? for instance, in case we want more buses and players?
In that case, I think it makes a lot of sense to add the dimensions to GameSettings, which is also an attribute of GameState. Thus, it would be accessible both to the GameInitializer and to the Frontend as it is required in both parts.
Regarding the Point class and the euclidean distance, I did notice those but decided not to use them directly from the frontend. We can refactor new_game.py once we decided where to put those useful geometric functions.
There was a problem hiding this comment.
I think a single map size makes sense, we don't want to have to scroll around to find stuff on the front end so ideally it would fit all on one screen
There was a problem hiding this comment.
map_size added to GameSettings
There was a problem hiding this comment.
As a general comment, you don't need to add comments to every function. Often things change and the comments are not updated and they end up containing outdated information so usually it is best to use them only when really needed to explain a relatively complex function.
Worst case is external documentation (Very hard to keep aligned with code)
Ok case is you need comments (Somewhat hard to keep aligned with code)
Best case is the code is clear enough to understand without comments
Try to describe clearly what the function does with the function name and the type hints if you can.
For example this idea of dict[str, BusId] means that you have to read the comment to know what to expect the strings to be which is kind of annoying. Also if you ever change the names of these strings you will have to change them in two places.
If you return a pair of buses as tuple[BusId, BusId] that is quite clear and just using the IDE type checker you can understand straight away what is inside the returned variable and how to access them without required any other information such as the strings "bus1" and "bus2".
Or you could even define BusIdPair = tuple[BusId, BusId] somewhere in the bus model file and then you can refer to the concept of a BusIdPair in the return type
There was a problem hiding this comment.
Also do you know if this code works? It would be good to have a unit test. It doesn't need to be super fancy but at least something that confirms you can run this code without it crashing