- Fix the rabbit AI
- Enable invoke_synced_command to also call (some) built-in commands
and give an error message in the case of an unknown command
- Remove some unnecessary implementation details
fixes#1649 . ai.synced_command could easily be used to implement all types of
undeteced cheats so it was removed. As a replacement this commit adds a
[custom_command] synced command that just calls
wesnoth.game_events.on_synced_command which calls a lua handler that
must first be set.
previously the unit ctor could invoke arbitrary umc lua code, for
example via advance_to -> add_modification ->
resources::lua_kernel->apply_effect and during that call the unit was
not owned by a unit_ptr, so the code could not use unit_ptr objects at
several places because the unit might not be owned by a unit_ptr object.
This also covers handling of legacy DescriptionWML for [set_menu_item],
and tweaks its handling in the other cases (mainly [multiplayer_side]).
I added the next_dev_version constant intending to use it, but then
decided not to do so; however I left it in in case someone finds it useful.
Looks like this was lost in the refactor that introduced the luaW_checklocation helper function.
If the location is specified as two separate parameters for x and y, this helper function
actually pops the y parameter from the stack so that further parameter tests can act
as if the location was a single parameter. Thus, the fire_event parameter, which used to
be 4th on the stack, is now moved down to 3rd.
Should restore the behavior lost in 09906135b5 where in attempting to assign a village
to a new side that was defeated will have no effect. My changes made it that the village
would still be unassigned from the old side in that case.
@gfgtdf
Turns out I forgot to consider the old village being unassigned, which would
make the exception on getting the old team a common occurrence. I've changed
the code to continue on if it can't unassign the village.
I also noticed a slight semantic change: unassignment of the old side used to
be skipped if the new was defeated, but now it's not. Dunno if that's a big deal.
@gfgtdf pointed out that I didn't actually fix the underlying porblem that was causing
#2505 in 3e9654c3e4: if new_side was 0, board().team_is_defeated(board().get_team(new_side))
would throw an out-of-range exception and the village would never be un-assigned from the
old side.
I'm totally refactored the function to be easier to understand and match the desired behavior.
The code wasn't checking for a case where new_side was 0... not sure why that didn't cause
a problem before. Anyway, since I made get_team() throw if the side was invalid, I made the
code catch the out-of-range exception. That way we don't have to worry if either new or old
side is invalid.
The only function that display meaningfully implemented was get_disp_context. It did implement
get_tod_man, but only meaningfully in its two derived classes (it returned the result of
resources::tod_manager in display). Additionally, the long-ass comment in the header stated the
display class should *not* inherit from filter_context. Do note that get_disp_context was retained
as a display member function.
Upon further investigation, I found that there were only two places where the display class was
passed to unit_filter. All others passed resources::filter_con. The editor_display class was also
assigned to resources::filter_con in editor::context_manager. So, I removed the second filter_context
argument from unit_filter and initialized it from resources::filter_con in all cases.
unit_filter only used the filter_context to get its display_context and the unit_map within anyway.
When using display::get_disp_context in-game, that would return a game_board object. Using
resources::filter_con in the same context would return the game_state object that owned the
aforementioned game_board, and calling get_disp_context on that game_state would return the board
as well. So we end up in the same place.
To be complete, I also made editor::context_manager inherit from filter_context. It makes much more
sense, since it can nicely implement 2/4 of the fc functions. It also ensures resources::filter_con
is valid in the editor, in case it's needed. I'm not 100% sure it is, but since it was assigned
before (to editor_display), it's very likely.
Another side effect of the above is that get_tod_man is now implemented in a more apropos class
in the editor. Essentially, editor_display (where it was implemented before by reaching into the
context_manager) holds an editor_controller which holds a context_manager (where it's now implemented).
It wasn't even really needed in editor_display and was only called once.
Similarly, get_tod_man has been removed from game_display. Again, it was only present to "properly"
implement the function in display, which only existed because display inherited from filter_context.
And get_tod_man wasn't even needed in display, game_display, or editor_display (save for the one
aforementioned call)!!! AND in game_display, it simply dereferenced a pointer to the *actual* ToD
manager in the game_state class, WHICH IN AND OF ITSELF INHERITS FROM FILTER_CONTEXT!!!
I have removed the tod_manager pointer in game_display and replaced its use with resources::tod_manager,
which in the context of the game (where game_display) would be use, point to the same thing the class
pointer did. I suppose I could have left it, since I'm trying to remove/improve the use of the
resources pointers, but I felt it better to decouple the two classes (game_display and game_state)
slightly, especially since its main purpose for existing (overriding display::get_tod_man) no longer
exists.
Finally, some of the instances where a unit_filter object is created had to be changed to use brace
initialization or else the build failed. @celticminstrel tells me this might be because of some "most
vexing parse" issue.
Some formatting and virtual/override specifiers were also applied/added.
*huffs*
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.
Also made conditions fail if they encountered a syntax or runtime error. This seems the
more logical behavior than passing.
WML conditional tags were split into their own Lua file. The one in lua/wml/object.lua
relies on local variables so was left there.
This a two part commit. First:
----------------------------------------------------------------------------------------------------
Added and deployed two new helper macros for the standard implementations of the static execute
and display functions. I also made use of a variadic template in order greatly simplify code
maintenance. Now, even if the dialog's ctor parameters change, no one has to worry about updating
the associated execute/display functions (though of course, this only applies if the helper macros
are used). \o/
I did not deploy the macro in cases where there were multiple overloads or the functions did more
than just show their dialogs. I might add an additional __VA_ARGS_ parameter to the macros later.
Do note for the end_credits dialog I moved the default empty-string parameter from the display
function to the ctor.
Second:
----------------------------------------------------------------------------------------------------
Another change is that modal_dialog::show and modeless_dialog::show no longer take CVideo arguments.
Since the video argument couldn't be included in the parameter pack (maintaining the argument would
have meant making it the first one, which would be just as much work), and using CVideo::get_singleton
in the macros would require adding video.hpp includes in a whole bunch of files, I simply removed the
argument. I had been intending to do this for a while anyway.
This therefor also removes the CVideo argument from:
* All dialog display/execute functions.
* modal_dialog::show
* modal_dialog::build_window
* modeless_dialog::show
* modeless_dialog::build_window
* wml_exception::show
* gui2::show_message
* gui2::show_error_message
* gui2::show_transient_message
* gui2::show_transient_error_message
* gui2::show_wml_message
* gui2::build
* gui2:🪟:window
* gui2::dialogs::tip::show
* Various GUI2-related Lua functions. The video_dispatch helper was also removed.
* Any functions that took a CVideo argument for the sole purpose of passing it to one of the above.
Ya know, all these damn CVideo arguments didn't actually do anything, besides an occasional check to
CVideo::faked. At the end of the pipeline, they just got assigned to the video_ member of gui2::window.
Huge code bloat for nothing.
We have needed to suppress it in a bunch of places because GCC has kept
giving false positives depending on optimization settings. When I noticed
yet another false positive when I attempted to build the game with PGO
profiling data using CMake, it was the last straw.
Let's just suppress the warning globally and move on with our lives.
Fixes halo remaining when a unit is killed with [put_unit] or replaced by
creating a new unit over it with the context menu, and duplicate halo when a
developer changes the properties with :unit.
This should fix#2021. The reason why we didn't just add one events::command_disabler in the code that starts the unsyned event hander (moving the events::command_disabler up in wml_menu_item::fire_event) is the we want commnds_disables to be for the (wml) code that handles the menu items so that it knows that it can use issue synced commands via [do_command]
this attribute return whether we are in a context where the user coudl issue new commands.
This is mainly intended to be used to check whethere we can use [do_command] in the currently situation (for exampel if one wants to use [do_command] in a unsycned menu item)
The cause for this bug was the whiteboard.
A unit destroys its halo when the unit itself is destroyed. Makes sense,
right? Unfortunately, units are managed by shared pointers, and therefore
are not destroyed until all references to them are gone. And the whiteboard
retains a reference to the most recently selected unit.
The obvious fix would be to use a weak (non-owning) pointer in the
whiteboard, but it's sadly not possible because unit_ptr is an
intrusive_ptr. It stores the reference count right in the unit itself, and
therefore the refcount is lost when the unit is destroyed: hence weak
pointers are impossible.
Thus, what I ended up doing was explicitly notifying the whiteboard when an
unit is killed, and releasing the reference when it happens. If anyone has
suggestions about a better implementation, I'd love to hear them.
this adds a new tag [cancel_action], it currently only has an effect in
move actions. It can be used in enter_hex/exit_hex events to abort the
current movement. It can also be used in moveto events where is
cancels possible following attacks (if the user issued a move+attack
action).
This also changes the behaviour of moveto and enter/exit_hex event so
that they now no longer cancel the movement automatically, also
[allow_undo] no longer has an effect on whether the move is cancelled,
it now only allows undoing of the move.
