Difference between revisions of "SoC Ideas Whiteboard Backend Refactoring 2012"
(→Additional Information) |
(→Related refactoring concerns) |
||
Line 46: | Line 46: | ||
* The whiteboard uses a mix of approaches to recover pointers to units that actions are related to. One that's very common in the rest of the game code is doing a unit map search on the hex the unit is supposed to be in (which we never know after any kind of game even took place, WML scenario code can erase and replace units at will). See for instance wb::move::apply_temp_modifier(). Another, more recent approach is to store and use the unit's underlying ID and use that to recover the unit pointer from the unit map. See wb::move::get_unit() for that approach. Leftovers of other approaches may exist. Evaluate which of the current approaches make the most sense, stress test it with unit tests, and uniformize unit access where possible. | * The whiteboard uses a mix of approaches to recover pointers to units that actions are related to. One that's very common in the rest of the game code is doing a unit map search on the hex the unit is supposed to be in (which we never know after any kind of game even took place, WML scenario code can erase and replace units at will). See for instance wb::move::apply_temp_modifier(). Another, more recent approach is to store and use the unit's underlying ID and use that to recover the unit pointer from the unit map. See wb::move::get_unit() for that approach. Leftovers of other approaches may exist. Evaluate which of the current approaches make the most sense, stress test it with unit tests, and uniformize unit access where possible. | ||
+ | |||
+ | * boost::dynamic_pointer_cast vs simple numeric id: we use dynamic_pointer_cast quite a bit, which is known to have quite an overhead. Our action class hierarchy is pretty small and is not gonna change a lot in the foreseeable future. Evaluate whether using some numeric type IDs such as wb::action::MOVE, wb::action::RECRUIT and so one combined with static pointer casts when appropriate would be better. | ||
==Playtesting== | ==Playtesting== |
Revision as of 02:40, 30 March 2012
This page is related to Summer of Code 2012 |
See the list of Summer of Code 2012 Ideas |
This is a Summer of Code 2012 Idea |
Description
Whiteboard: Refactor the backend
Page for the idea: SoC Ideas Whiteboard Backend Refactoring 2012
Improve the whiteboard's (the wesnoth action-planning system) backend code by refactoring the data structure which holds the planned actions. Also refactor the verification and mapbuilding processes by combining them, and use this single new process to cache as much data as possible to avoid redundant lookups. You can also propose other simplifications.
Produce complete unit tests to cover all operations on the new planned action storage and the new combined mapbuilding/verification.
Document this new backend, explaining your design approach and including all diagrams necessary for the reader's understanding.
Finally, do exhaustive playtesting and bugfixing involving devs and community members to make sure the whiteboard is fully reliable.
There are 2 submitted student proposals for this idea
Étienne Simon - Whiteboard backend polishing
My project is to make the Whiteboard code cleaner and to redesign small parts of it to speed it up. The global design of the Whiteboard won't be changed a lot, each part will be reviewed individually. I'm not only planning to improve the Whiteboard backend, but also to document the overall design and each part of it as well as to write a wide variety of test to improve its stability. Moreover, I'll factorise action handling outside of the Whiteboard so that the same code will be usable in all Wesnoth.
See Ejls/GSoC 2012/Whiteboard Backend Refactoring for more information.
John Gamboa - Whiteboard Backend Refactoring
There is a lack of optimization and consistency between the mapbuilder and the validator. These two classes big "entities" are intended to be integrated in one only mapbuilder-validator. Since the validator asks for the information available in the mapbuilder frequently, this should provide more efficience and solve the consistency problems.
In a 4 months based approach, a very preliminar cronogram shall be:
- integrate the mapbuilder and the validator - 2 months
- test, bugfix and document the changes - 2 month
Initially, I can't give have a better calendar.
See Soc2012 vaulttech refactor the backend for more information.
Additional Information
Combining the mapbuilding and action verification
The whiteboard planning system can show you things like possible enemy moves "in the future", after you've moved your units to the planned positions. To achieve this, it builds an internal future state by taking the orders of the player and applying them to the game state in the correct order, a process we call "mapbuilding".
The issue here is that we also have a separate "verification" of said orders to make sure they are possible. This is used everytime the game state might have changed outside of the whiteboard's control, or whenever the player deletes one of his planned actions, and so on. For historical reasons this is separate from mapbuilding, which is a problem since we end up with a lot of duplicate code (for instance compare whiteboard/move.cpp line 369 and whiteboard/validate_visitor.cpp line 87 as of svn revision 53682, both do a unit map find to check for the unit's presence), and since they don't handle actions spread over multiple turns consistently.
Another issue is that we often do a full search of the planned actions for various purposes, which is not optimal.
The solution to both of those is to combine mapbuilding and verification in a single process, so that we basically validate actions every time we build the future state. We also want to take advantage that we're going over every planned action to cache some data, such as which hexes are planned moves destinations: this way we avoid going over the planned actions again when displaying the whiteboard's visual aids.
Lastly we also have a "highlighting" process which identifies which planned actions should receive a visual highlight depending on the mouse position. A lot if not all of the information processed there should be instead cached inside the combined mapbuilding/validator.
Refactoring the actions container
Right now the actions for each turn are held in a separate stl container, and we have an iterator front-end to make all of a player's actions accessible as one container. This makes for an overly complicated and wasteful backend. The objective here is to simplify it as much as possible and make it rock-solid.
Related refactoring concerns
- We kind of have a mix of two approaches, one where the action object does the operations itself (mapbuilder calls the action's apply/remove_temp_modifier() ) and one where a friend class does all the work to keep the action classes smaller (validator is friend to all actions and I think it still accesses some of their fields directly). Consider whether moving completely to either approach makes sense.
- The whiteboard uses a mix of approaches to recover pointers to units that actions are related to. One that's very common in the rest of the game code is doing a unit map search on the hex the unit is supposed to be in (which we never know after any kind of game even took place, WML scenario code can erase and replace units at will). See for instance wb::move::apply_temp_modifier(). Another, more recent approach is to store and use the unit's underlying ID and use that to recover the unit pointer from the unit map. See wb::move::get_unit() for that approach. Leftovers of other approaches may exist. Evaluate which of the current approaches make the most sense, stress test it with unit tests, and uniformize unit access where possible.
- boost::dynamic_pointer_cast vs simple numeric id: we use dynamic_pointer_cast quite a bit, which is known to have quite an overhead. Our action class hierarchy is pretty small and is not gonna change a lot in the foreseeable future. Evaluate whether using some numeric type IDs such as wb::action::MOVE, wb::action::RECRUIT and so one combined with static pointer casts when appropriate would be better.
Playtesting
Past experience has shown that properly testing the whiteboard means testing:
- campaigns
- multiplayer scenarios vs the AI and vs players, with and without heavy WML scripting
- observers (can cause bugs and should be tested)
- 2v2 matches
For proper playtesting you'll need to create several very simple scenarios that allow to quickly test all the possible planned actions, i.e. recruit, recall, move (including over several turns), attack, and eventually the "suppose dead" action if we re-enable it.
You should at first do exhaustive tests by yourself up to simulating 2v2 matches with 4 local clients, and when you're confident the game is bug-free, organise matches with devs and community members to put the whiteboard's usability to the test, and to find the last, least obvious bugs.
Requirements
Good skills with the C++ stl, and boost shared pointers, are recommended. As is basic experience in writing unit tests. Prior Wesnoth multiplayer experience is a plus, but not essential.
Since this project is very focused you're expected to have a pretty complete view of what you want to develop down to the most important class members, before GSoC starts. You're also expected to provide a realistic development calendar.
Note that all development must be finished by the GSoC midterm evaluation, after which all the time will be reserved for documenting, playtesting and bugfixing.
Whom to ask about this
gabba on irc. If I'm hard to reach you can also find my email in the game's source files (if you're gonna make the effort to find it, I can make the effort to read your mail ;) ).