[wesnoth-commits] [wesnoth/wesnoth] 86f430: Refactoring movetype and terrain_costs (#2486)

[Steve Cotton]

  Branch: refs/heads/master
  Home:   https://github.com/wesnoth/wesnoth
  Commit: 86f430c2ddf6412726989e4ff4a705fe46c19e30
      https://github.com/wesnoth/wesnoth/commit/86f430c2ddf6412726989e4ff4a705fe46c19e30
  Author: Steve Cotton <steve at octalot.co.uk>
  Date:   2019-07-31 (Wed, 31 Jul 2019)

  Changed paths:
    M src/actions/vision.cpp
    M src/actions/vision.hpp
    M src/movetype.cpp
    M src/movetype.hpp
    M src/units/types.cpp
    M src/units/unit.cpp

  Log Message:
  -----------
  Refactoring movetype and terrain_costs (#2486)

Simplify the public interface for movement, vision and jamming costs to only
the movetype class and a read-only interface called "terrain_costs".  Changes
to terrain_costs now need to be done through the movetype instance, so logic
about how movement, vision and jamming interact with each other can be handled
in movetype.  This fixes a possible dangling-pointer issue: the default
copy/move implementation in movetype::terrain_info would have copied raw
pointers, without ensuring that those pointers remained valid.

This feels too complex, but also feels like time to merge it and do any further
cleanup as a separate commit.  For example, there's only one place that calls
make_data_unique, and one place that calls make_data_shareable. Merging the
functions should make it clearer.

The terrain_info class is now a private class implementing the terrain_costs
interface, previously it was a public class with raw pointers to other
instances of itself, and a copy constructor which copied the raw-pointers
as-is. One of the raw pointers is still there, but it's only non-null when both
instances involved are owned by one instance of movetype.

=== To cherry-pick back to 1.14 ===

Cherry-pick the tests from 358f564301273f32338a00508f5121063ae800b1.

* units.cpp: needs a manual merge. Remove the `set_attr_changed(UA_MOVEMENT_TYPE)`
  call from the master version.
* movetype.cpp: take the changes in this commit, in each case the conflict is
  the change of std::shared_ptr::unique() to std::shared_ptr::count(), which
  has been replaced with make_data_shareable() and make_data_writable().
* movetype.hpp: take the changes in this commit, deleting the old terrain_costs
  class and adding functions to movetype.