Remove the serialization of colors in Minimap. It serializes them only when they're the player color, but we serialize the player color in the Player component anyway, so it seems unnecessary. None of the other components that use color serialize it. The only consequence is that on deserialization we don't have access to the colors anymore, so we have to set them to a dummy color and then set them to the real colors after everything's been deserialized. That just means we need to call a function to set the color on session init, easy enough.
Details
- Reviewers
bb - Commits
- rP20797: Stop serializing minimap playercolors
- Trac Tickets
- #3834
#4747
Test that the minimap colors are correct when loading saved games and on multiplayer late joins.
Note that in Atlas after changing the player color when you 'Play' again the minimap unit colors will be correct. (Previously they stayed the original colors even after resetting.)
I used the generic UpdateDisplayedColors because the plan with D754 is to update the colors of other components too, and I'd like to use the same function.
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 3996 Build 7007: Vulcan Build Jenkins Build 7006: arc lint + arc unit
Event Timeline
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
source/simulation2/components/ICmpMinimap.cpp | ||
---|---|---|
1 | 2017 |
Patch looks ok to me, tested rejoining a game (on same pc), and no OOS occurred, code looks same, but I am not authority on it, leaving it open a bit in case someone with more experience with these parts of the code wants to yell.
(only posting the inline comments from last time I looked at it)
binaries/data/mods/public/gui/session/session.js | ||
---|---|---|
279 | (Ok to have it here because it is planned to call that more often than once) | |
binaries/data/mods/public/simulation/components/GuiInterface.js | ||
869 | (Minimap entities sounds a bit like a set of Entities specifically crafted for the Minimap independent from the units and structures) | |
871 | UpdateDisplayedPlayerColors to give the player context. | |
885 | (entities can be inlined) | |
source/simulation2/components/CCmpMinimap.cpp | ||
222 | Passing the owner as an argument means that the function has the freedom to be called with a wrong owner. | |
238 | (I was looking for some const to replcae the 255 with a reference to u8 for fun (lib/types.h:typedef uint8_t u8) http://www.cplusplus.com/reference/cstdint/) but didn't succeed. anyway.) |
don't pass owner
source/simulation2/components/CCmpMinimap.cpp | ||
---|---|---|
222 | I was never comfortable with that. |
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Performed a quick oos test on two instances on the same machine, didn't notice an oos (not that that is a proof though).
Serialization has been added in rP8449
binaries/data/mods/public/simulation/components/GuiInterface.js | ||
---|---|---|
873 | inlinable? | |
874 | nukable | |
source/simulation2/components/CCmpMinimap.cpp | ||
1 | Happy new year!! | |
191 | why did we do that? serializing stuff? |
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Atlas, saving, loading front doesn't fall,
captured trees/mines do not use the player color, also after saving and loading
oos rejoin test, still doesn't fail
source/simulation2/components/ICmpMinimap.cpp | ||
---|---|---|
1 | gotta luv this |