From 595c4ec542174527772aef387394a909c14d6c4e Mon Sep 17 00:00:00 2001 From: Sveinung Kvilhaugsvik Date: Mon, 8 Feb 2021 16:38:08 +0100 Subject: [PATCH 3/3] Lua API: remove some move unit side effects. Unit:move() and Unit:teleport() accumulated many exceptions to enabler defined rules as more and more parts of regular unit move can under action enabler control. They were able to conquer cities, embark into transports, conquer extras and pop huts. The reason for the exceptions was strict backwards compatibility. Freeciv can't know if a ruleset or scenario author intended a move as what now was an enabler controlled action. The action side effect of the move may have been intended by, irrelevant to or overlooked by the script author. Keeping this stirct backwards compatibility made it difficult to move certain rules to the ruleset. The best that could be done for them was obligatory hard requirements.This is why UTYF_CIVILIAN - to take one example - hasn't moved to the ruleset yet. The exceptions can also make a ruleset author - assuming a call to unit_move() is a regular move - accidentally trigger behavior currently controlled by actions and end up breaking his own rules in a Lua script. A unit can now be forced to perform an action - as long as it is legal - with Unit:perform_action(). Given the above I say that the time has come to purge some side effects. Remove the city conquest, extra conquest and hut popping side effects. Allow embarking to stay for now. It casues litte trouble and someone may use one of the functions to teleport into a transport or something like that. See osdn #41523 --- server/scripting/api_server_edit.c | 50 +++++++----------------------- 1 file changed, 11 insertions(+), 39 deletions(-) diff --git a/server/scripting/api_server_edit.c b/server/scripting/api_server_edit.c index c7f8a0dbd9..ac17c71520 100644 --- a/server/scripting/api_server_edit.c +++ b/server/scripting/api_server_edit.c @@ -159,7 +159,6 @@ Unit *api_edit_create_unit_full(lua_State *L, Player *pplayer, Tile *ptile, bool api_edit_unit_teleport(lua_State *L, Unit *punit, Tile *dest) { bool alive; - struct city *pcity; LUASCRIPT_CHECK_STATE(L, FALSE); LUASCRIPT_CHECK_ARG_NIL(L, punit, 2, Unit, FALSE); @@ -172,26 +171,14 @@ bool api_edit_unit_teleport(lua_State *L, Unit *punit, Tile *dest) * remove auto embarking completely or for transports * the unit can't legally board. -- Sveinung */ NULL, TRUE, - /* Backwards compatibility for old scripts in rulesets - * and (scenario) savegames. I have no objection if you - * see the old behavior as a bug and remove auto - * conquering completely or for cities the unit can't - * legally conquer. -- Sveinung */ - ((pcity = tile_city(dest)) - && (unit_owner(punit)->ai_common.barbarian_type - != ANIMAL_BARBARIAN) - && uclass_has_flag(unit_class_get(punit), - UCF_CAN_OCCUPY_CITY) - && !unit_has_type_flag(punit, UTYF_CIVILIAN) - && pplayers_at_war(unit_owner(punit), - city_owner(pcity))), - (extra_owner(dest) == NULL - || pplayers_at_war(extra_owner(dest), - unit_owner(punit))) - && tile_has_claimable_base(dest, unit_type_get(punit)), - unit_class_get(punit)->hut_behavior != HUT_FRIGHTEN); + /* Keep unit_move mostly about movement, not as a way to + * accidentally bypass action enabler defined rules. A + * modder that wishes to make a unit perform an action + * from Lua can use Unit:perform_action() -- Sveinung */ + FALSE, FALSE, FALSE); if (alive) { struct player *owner = unit_owner(punit); + struct city *pcity = tile_city(dest); if (!can_unit_exist_at_tile(&(wld.map), punit, dest)) { wipe_unit(punit, ULR_NONNATIVE_TERR, NULL); @@ -809,8 +796,6 @@ void api_edit_player_victory(lua_State *L, Player *pplayer) bool api_edit_unit_move(lua_State *L, Unit *punit, Tile *ptile, int movecost) { - struct city *pcity; - LUASCRIPT_CHECK_STATE(L, FALSE); LUASCRIPT_CHECK_SELF(L, punit, FALSE); LUASCRIPT_CHECK_ARG_NIL(L, ptile, 3, Tile, FALSE); @@ -822,24 +807,11 @@ bool api_edit_unit_move(lua_State *L, Unit *punit, Tile *ptile, * remove auto embarking completely or for transports * the unit can't legally board. -- Sveinung */ NULL, TRUE, - /* Backwards compatibility for old scripts in rulesets - * and (scenario) savegames. I have no objection if you - * see the old behavior as a bug and remove auto - * conquering completely or for cities the unit can't - * legally conquer. -- Sveinung */ - ((pcity = tile_city(ptile)) - && (unit_owner(punit)->ai_common.barbarian_type - != ANIMAL_BARBARIAN) - && uclass_has_flag(unit_class_get(punit), - UCF_CAN_OCCUPY_CITY) - && !unit_has_type_flag(punit, UTYF_CIVILIAN) - && pplayers_at_war(unit_owner(punit), - city_owner(pcity))), - (extra_owner(ptile) == NULL - || pplayers_at_war(extra_owner(ptile), - unit_owner(punit))) - && tile_has_claimable_base(ptile, unit_type_get(punit)), - unit_class_get(punit)->hut_behavior != HUT_FRIGHTEN); + /* Keep unit_move mostly about movement, not as a way to + * accidentally bypass action enabler defined rules. A + * modder that wishes to make a unit perform an action + * from Lua can use Unit:perform_action() -- Sveinung */ + FALSE, FALSE, FALSE); } /*************************************************************************//** -- 2.20.1