Update of patch #1538 (project wesnoth):

                  Status:                    None => In Progress            
             Assigned to:                    None => crab                   

    _______________________________________________________

Follow-up Comment #1:

thanks for the patch! a few comments:
1)

if(un.valid() && un->second.type_id() == u.type_id()){ 

if WML event changes the type of a recruited unit, this is normal and allowed
situation. It will not break the checksum, since (hopefully), the other side
will handle the recruit event in the same way, changing the type of the unit,
too. So, "un->second.type_id() == u.type_id())" is not needed.

2)

else { return false; }

we usually split it to separate lines, like this :

else { 
    return false; 
}


generally, (1) and (2) can be replaced just by "return un.valid()"
but, see also the following:

3)

fgrep -Rn add_checksum_check ./src
./src/ai/actions.cpp:763:               
recorder.add_checksum_check(recall_location_);
./src/menu_events.cpp:804:      recorder.add_checksum_check(loc);
./src/menu_events.cpp:975:      recorder.add_checksum_check(loc);
./src/menu_events.cpp:1142:                             
recorder.add_checksum_check(loc);
./src/menu_events.cpp:1185:                     
recorder.add_checksum_check(loc);
./src/replay.cpp:405:void replay::add_checksum_check(const map_location&
loc)
./src/replay.hpp:68:    void add_checksum_check(const map_location& loc);

as you can see, add_checksum_check is called in more places than you've
patched. maybe it's better to patch add_checksum_check to check the unit
iterator validity, instead of patching all the callers ?


    _______________________________________________________

Reply to this item at:

  <http://gna.org/patch/?1538>

_______________________________________________
  Message sent via/by Gna!
  http://gna.org/


_______________________________________________
Wesnoth-bugs mailing list
[email protected]
https://mail.gna.org/listinfo/wesnoth-bugs

Reply via email to