Follow-up Comment #2, patch #2640 (project wesnoth):
Hi nice to see a large patch of a GSoC applicant.
I had a look at the patch, since I'm not familiar with whiteboard I didn't
review how good it fits in wb, I leave that to gabba. I did look at the code
itself. Pleasantly surprised by the amount of comment in the source files.
Below some issues I found in your patch.
You added a new hotkey, but didn't register a default in
data/core/hotkeys.cfg.
Since you also do not allow modification of the hotkey the feature has no
hotkey assigned.
We use /** */ for doxygen comment for consistency please also use that. I
saw
later that you do that at most places, guess first run into one in another
style.
The assert in the constructor of suppose_dead is what odd. If it triggers
you're probably dead since you deferred the pointer in the member
initialization list. If the pointer can't be NULL best use a reference else
you
should check in the initialization list before deferring.
>From a quick glimpse it's used from playsingle_controller where you defer an
iterator and take the address of the unit, so changing the interface to
references might be the easiest.
In general we try to send references if the item must exist and a pointer if
it
might be a `NULL'.
In whiteboard/validate_visitor.cpp you added a
// to circumvent a bug in std::equal()
When doing so please add more information, especially on which compiler you
ran
into that issue.
Just curious, in whiteboard/suppose_dead.hpp you add a `scope' for what you
inherited from wb::action. Did you do that on purpose?
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?2640>
_______________________________________________
Message sent via/by Gna!
http://gna.org/
_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs
