Refactor client game code#138
Conversation
StenAL
left a comment
There was a problem hiding this comment.
Looks mostly good and the changes are really nice since it untangles a lot of the complexity in the codebase!
One issue I have is that some commits fail the build. This makes bisecting issues difficult in the future since this project uses merge commits which preserves your commit history.
It would be great if you could fix that, either by squashing commits or by rebasing and fixing them one-by-one. For reference, I used git rebase upstream/master -x "mvn clean && mvn install" to test whether each commit passes the build.
shared/src/main/java/org/moparforia/shared/tracks/parsers/VersionedTrackFileParser.java
Outdated
Show resolved
Hide resolved
shared/src/main/java/org/moparforia/shared/tracks/parsers/VersionedTrackFileParser.java
Outdated
Show resolved
Hide resolved
Create new classes Track and Map and extract related game logic to them.
6508786 to
31f20e2
Compare
|
@StenAL Thanks for the review and sorry for the slow response. I did In addition, I reworded the commit messages to better align other commit messages in the history. |
StenAL
left a comment
There was a problem hiding this comment.
Looks really good, thanks for this!
This is great work since it's deobfuscating the most complex part of the whole application. Everything looks way more understandable and decoupled than before.
|
Thank you for the awesome contribution. I really appreciate the efforts. |
This PR does a major refactor to client side game code. Goal is to make code more readable and maintainable, not to change any existing behavior. I have done a fair amount of testing and have not noticed any changes. However, at least when I tried to simplify
getFriction, it resulted in different behavior and I reverted the change. If you experience any changes, please let me know and I will fix it.Sorry for huge PR, I considered about splitting it into two but was not sure if that would achieve anything. Following the changes is little complicated due to renaming variables and moving code around, so thought that it might be simplest to just put everything into one PR, especially when the track parsing was removed completely from client. If you want, I will break this into multiple PRs.
Features:
Other notes:
Game,Shot, and some kind of interface for rendering graphics.Advances issue #69