Update of patch #746 (project wesnoth):
Assigned to: None => boucman
_______________________________________________________
Follow-up Comment #3:
ok, here are my thoughts on your patch
overall, I like it and I would gladly commit it "as is"
however, I can see a few improvement that could make it go from "good" to
"great"
* I dislike the idea of adding a member to units just for cycling, units are
independant of the position (theoretically)
* I dislike the idea of having that code tied to "mouse event" it doesn't
belong there. You only have it here to reuse the unit_ member, but if we want
to make it more generic, we will have to use other unit maps anyway...
* the most logical unit map to use is the one in game_display, since these
are the units on the map, you should either put your code there or in a
separate class
* the code for healing is in actions.cpp, the function is called
calculate_healing
so, here is an idea of how it could be done more cleany
you should tie your code to the unit_map class (svn only, probably was not
there when you originally provided the code) as a special iterator in that
class. That would allow easy use and integration in existing code...
tell me what you think
_______________________________________________________
Reply to this item at:
<http://gna.org/patch/?746>
_______________________________________________
Message posté via/par Gna!
http://gna.org/
_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs
