Also did some formatting cleanup in game_board. Didn't really care to touch the others.
Removed virtual specifier from non-const game_board::teams. It didn't override anything
in the base class (display_context, which has the const version) and nothing inherits
from game_board to override it (the non-const version) later.
cppcheck (via Codacy) notes that it's not clear that the iterator (j) cannot exceed end() because most don't expect pointer math.
Rewrote to make it more clear what is going on.
cppcheck (via Codacy) had a problem with the form `T & T` so used `auto & T`
This is probably a bug in cppcheck but reported to Codacy in case it's their fault since the message form was for && and this was just &.
cppcheck (via Codacy) pointed out that a variable was initialized, then reassigned, before the initialed value was used.
Moved the variable declaration to the point the actually-used value was assigned.
cppcheck (via Codacy) rightly noted that the construct ((!!w ^ !!h)) was confusing.
Rearrange the code a bit, note some things must be true without testing them, and we can eliminate two variables, make the code faster than the "cute optimization" and make it FAR easier to read.
cppcheck (via Codacy) reports the lowest-quality C++ is src/sever/game.hpp with 41 warnings.
There is no need to copy the const socket_ptr struct all over place to pass-by-value. It's const, so we can pass-by-reference and save time and memory.
Map Context:
* Removed get_team() in favor of non-const local team() overload.
* Removeded get_map() (both overloads) in favor of map() and a local non-const overload of the same.
* Made both overloads of map() return editor_map instead of gamemap (former inherits from the latter).
Context Manager:
* Removed team, unit, and label accessors that only fetched the same info from the current map_context.
Having more functions of this name only made things a lot more confusing.
Editor Controller:
* Change all instances of the three intermediate accessors mentioned above to data queries directly
from the current map context via editor_controller::get_current_map_context. The result is the same,
we just no longer have three levels of indirection.
I had probably added this before since I wanted a player joining a game to always choose
a leader, but that's rather mitigated by the fact that there's a cancel button.
This used to need to be a class of its own in order to store some info prior to switching
the context, but that's not the case anymore. Moved map_context_refresher::refresh into
context_manager. Also moved the set_window_title call into the function as well, since it
was called in all three places the context refresher was used.
This reverts commit 0f8e25f970ee9b8fba010c5ffa528aa6238d8900. For some damn reason
this was causing invalid access crashes for me on VS 2017 release builds, but no one
else could repro. Since I can implement a non-const accessor in the map_context class
anyway, I'm reverting.
I had re-added it in a limited context in eaea9be1177398b2fe1 since I needed it to fix
(IIRC) a crash in the editor when adding units, since the editor didn't have a game_board.
However, looking at the code again, I realized that the display class (base of editor_display)
holds a pointer to a display_context object. map_context inherits from display_context, and
the editor sets the editor_display's display_context to the current map_context. Therefor, I
could just access the units needed via display::get_units, instead of needing a global pointer.
Could have been using the display singleton all this time, but it became a double cast
with the change from resources::screen to the game_display singleton.
This is consistent with the use of display::get_singleton() (in fact, it's the same
pointer). It also makes the code more readable, and means we get to further clean up
the resources set.
Note that I added the exclude-if-not-empty code directly to time_of_day::write, since
that's what the editor uses, instead of manually writing keys as it does with items and
side units. Hopefully that won't have an adverse effect somewhere else.
`#wesnoth.unit_types` should simply return the number of unit types.
This fixes the crash and reports the correct value: the number of unit types defined.
This does not fix the rest of the crap like pairs() crawls, you can't interate variations or get the variation count.
My knee-jerk reaction is to delete all scripting support for unit_types so that it's filed where it belongs: in the Trash. But, what the hell, nobody has complained about what utter shit this code is, so I'll just fix this so we don't crash and carry on working on getting the code to actually do what it's supposed to instead of the lame-ass shit it currently does.
Can you tell what I think of this file?
closes#2381
* The editor_icon= key will now be respected in the Unit Preview Pane
* If no appropriate icon is found, a generic icon will be used instead, in both the editor
and unit preview pane.
There's no AI manager in the editor (a game state is required), nor a need for one, so I simply
disabled the AI initialization if we're not in the editor.
Fixes#2372.
It turned out that the AI kept dangling references to the old Lua state,
and crashed while destroying AI contexts for destroyed sides.
The best way to avoid it is to ensure that game_state, which already owns
the Lua state, also owns the AI. That way, the AI will be destroyed before
the Lua state and a dangling reference can't stay.
