1.16: general problems caused by transport ships unloading their passengers

[cooljeanius]

This wose has been unloaded from this transport ship:

However, the image is still showing as being overlaid atop the ship that just unloaded it. This only started happening when porting to BfW 1.16; it worked fine on 1.14.
Edit: the original issue reported here with the unit image overlay is fixed; I'm keeping this issue open for other related issues I noted in follow-up comments...

[cooljeanius]

The macro {TEG_SHIP_PASSENGERS} is defined in utils/units.cfg:

The_Earths_Gut/utils/units.cfg

Lines 187 to 404 in 516617b

	#define TEG_SHIP_PASSENGERS
	[event]
	name=moveto
	first_time_only=no
	[filter]
	side=1
	[filter_adjacent]
	#ifdef __UNUSED__
	side=1
	type=Transport Ship
	#endif
	role=accepts_passenger
	[/filter_adjacent]
	[not]
	type=Battleship,Galley,Transport Ship,Pirate Ship
	[/not]
	[/filter]
	#ifdef DEBUG_MODE
	{DEBUG_MSG 1}
	#endif
	#I don't know how to access the triggering ship, it seems not possible with second_unit, so manually detect one of the triggering ships (or THE triggering ship)
	#this filter must succeed, otherwise the event wouldn't have triggered
	[store_unit]
	variable=u_SecondUnit
	[filter]
	[filter_adjacent]
	id=$unit.id
	[/filter_adjacent]
	#ifdef __UNUSED__
	type=Transport Ship
	side=1
	#endif
	role=accepts_passenger
	[/filter]
	[/store_unit]
	{CLEAR_VARIABLE u_SecondUnit.overlays}
	#ifdef DEBUG_MODE
	{DEBUG_MSG "($u_SecondUnit.x|,$u_SecondUnit.y|)"}
	#endif
	#store the unit to carry to the variables of the carrying ship
	[store_unit]
	variable=u_SecondUnit.variables.u_Passenger
	[filter]
	id=$unit.id
	[/filter]
	kill=yes
	[/store_unit]
	{VARIABLE u_SecondUnit.variables.u_Passenger.moves 0}
	#ifdef DEBUG_MODE
	{DEBUG_MSG $u_SecondUnit.id}
	{DEBUG_MSG $u_SecondUnit.canrecruit}
	#endif
	{VARIABLE u_SecondUnit.role has_passenger}
	#ifdef DEBUG_MODE
	{DEBUG_MSG $u_SecondUnit.variables.u_Passenger.id}
	#endif
	#store the ship's original id to its variables (that's the only key that's to be changed and that it had before)
	{VARIABLE u_SecondUnit.variables.s_OriginalId $u_SecondUnit.id}
	#ifdef DEBUG_MODE
	{DEBUG_MSG $u_SecondUnit.variables.s_OriginalId}
	#endif
	#change the ship's id, name, overlays, profile and canrecruit to the ones of the carried unit
	{VARIABLE u_SecondUnit.id $unit.id}
	{VARIABLE u_SecondUnit.name $unit.name}
	{VARIABLE u_SecondUnit.overlays $unit.overlays}
	{VARIABLE u_SecondUnit.profile $unit.profile}
	{VARIABLE u_SecondUnit.canrecruit $unit.canrecruit}
	#overwrite the unit on the map with the modified one
	[unstore_unit]
	variable=u_SecondUnit
	find_vacant=no
	[/unstore_unit]
	#apply an overlay using the carried unit's base image
	[unit_overlay]
	x,y=$u_SecondUnit.x,$u_SecondUnit.y
	image=$unit.image
	[/unit_overlay]
	{CLEAR_VARIABLE u_SecondUnit}
	#ifdef DEBUG_MODE
	{DEBUG_MSG 3}
	#endif
	[/event]
	[event]
	name=prestart
	[set_menu_item]
	id=unload
	description=_"unload passenger"
	[show_if]
	[have_unit]
	side=1
	x,y=$x1,$y1
	role=has_passenger
	[filter_location]
	[filter_adjacent_location]
	terrain=!,Wo
	count=6
	[/filter_adjacent_location]
	[/filter_location]
	[/have_unit]
	[/show_if]
	[command]
	#ifdef __UNUSED__
	[store_unit]
	variable=u_Unit
	[filter]
	x,y=$x1,$y1
	[/filter]
	[/store_unit]
	[message]
	id=$u_Unit.id
	# po: this is unused; don't bother translating:
	message=_"bla"
	[/message]
	#endif
	{CLEAR_VARIABLE unit.role}
	#ifdef DEBUG_MODE
	{DEBUG_MSG $unit.variables.s_OriginalId}
	{DEBUG_MSG $unit.overlays}
	{DEBUG_MSG $unit.canrecruit}
	#endif
	#change the carriers id back to its original id
	{VARIABLE unit.id $unit.variables.s_OriginalId}
	#ifdef DEBUG_MODE
	{DEBUG_MSG $unit.id}
	#endif
	{CLEAR_VARIABLE unit.variables.s_OriginalId}
	#clear the other 4 keys it hadn't had previously
	{CLEAR_VARIABLE unit.name}
	{CLEAR_VARIABLE unit.overlays}#removes also the overlayed unit image
	{CLEAR_VARIABLE unit.profile}
	{CLEAR_VARIABLE unit.canrecruit}
	#unstore the original carried unit (it hasn't been changed :))
	[unstore_unit]
	variable=unit.variables.u_Passenger
	find_vacant=yes
	x,y=$unit.x,$unit.y
	[/unstore_unit]
	{CLEAR_VARIABLE unit.variables.u_Passenger}
	#overwrite the unit on the map with the modified one
	[unstore_unit]
	variable=unit
	find_vacant=no
	[/unstore_unit]
	#ifdef __UNUSED__
	{CLEAR_VARIABLE u_Unit}
	#endif
	[/command]
	[/set_menu_item]
	#unless the player activates it, a transport ship doesn't catch units moving next to it
	[set_menu_item]
	id=accept_passenger
	description=_"accept passenger"
	[show_if]
	[have_unit]
	side=1
	x,y=$x1,$y1
	type=Transport Ship
	[not]
	role=has_passenger
	[/not]
	[not]
	role=accepts_passenger
	[/not]
	[/have_unit]
	[/show_if]
	[command]
	{VARIABLE unit.role accepts_passenger}
	[unstore_unit]
	variable=unit
	[/unstore_unit]
	[unit_overlay]
	x,y=$x1,$y1
	image="misc/accept-passenger.png"
	[/unit_overlay]
	[/command]
	[/set_menu_item]
	#allow to deactivate acceptance of passengers
	[set_menu_item]
	id=deny_passenger
	description=_"deny passenger"
	[show_if]
	[have_unit]
	x,y=$x1,$y1
	role=accepts_passenger
	[/have_unit]
	[/show_if]
	[command]
	{CLEAR_VARIABLE unit.role}
	[unstore_unit]
	variable=unit
	[/unstore_unit]
	[remove_unit_overlay]
	x,y=$x1,$y1
	image="misc/accept-passenger.png"
	[/remove_unit_overlay]
	[/command]
	[/set_menu_item]
	[/event]
	[event]
	name=victory
	[clear_menu_item]
	id=unload,accept_passenger,deny_passenger
	[/clear_menu_item]
	[/event]
	#enddef

[cooljeanius]

ok it looks like it ought to get removed at line 326?

The_Earths_Gut/utils/units.cfg

Line 326 in 516617b

{CLEAR_VARIABLE unit.overlays}#removes also the overlayed unit image

[cooljeanius]

ok it looks like it ought to get removed at line 326?

The_Earths_Gut/utils/units.cfg

Line 326 in 516617b

{CLEAR_VARIABLE unit.overlays}#removes also the overlayed unit image

So, no, that doesn't do it anymore; it looks like I'm going to have to use [remove_object] instead, but to do that I'll need to figure out how to get the name of the object I want to remove...

[cooljeanius]

ok so I think this is the last bug blocking a release on the BfW 1.16 add-on server...

[cooljeanius]

The original issue may be fixed now, but there's still some glitchiness in this area with recruitment flags and stuff, so I'm going to reopen

[cooljeanius]

...also, ships and passengers still have their events tied together even after unloading, meaning that if Hamel's transport dies after unloading him, it can still cause defeat.

[cooljeanius]

...and now that I've reloaded to a previous save, all the passengers are gone? wtf

[cooljeanius]

Maybe @CelticMinstrel has some more help for me after previously helping me in 9117fcc?

[cooljeanius]

Maybe @CelticMinstrel has some more help for me after previously helping me in 9117fcc?

...or maybe @Toranks can help?

[CelticMinstrel]

Overlays are converted to objects in 1.16. I presume that's the core issue here. You could inspect the resulting objects with :inspect to see if they have an ID, and use that with [remove_object]. You could explicitly use objects for the overlays so you control what the ID is. Or if you don't have other overlays you could use Lua such as the following to remove all overlays:

the_unit:remove_modifications{
    wml.tag.effect{
        apply_to=overlay
    }
}

Note: That'll remove any object that uses an overlay. It could easily remove a bunch of things you don't want it to.

[cooljeanius]

Overlays are converted to objects in 1.16. I presume that's the core issue here. You could inspect the resulting objects with :inspect to see if they have an ID, and use that with [remove_object].

OK, so I'm finally getting around to looking into this, and, this would be a lot easier if I could select the text from the inspector and copy it and paste it here... maybe I'll open a separate issue against upstream Wesnoth about that... anyways, for now, I guess a screenshot will have to do:

...I guess these are the relevant objects? One of them doesn't even have an ID...

You could explicitly use objects for the overlays so you control what the ID is. Or if you don't have other overlays you could use Lua such as the following to remove all overlays:

the_unit:remove_modifications{
    wml.tag.effect{
        apply_to=overlay
    }
}

Note: That'll remove any object that uses an overlay. It could easily remove a bunch of things you don't want it to.

[cooljeanius]

...oh wait, I wonder if wesnoth/wesnoth#5190 might be something I need to look out for if I'm going to be using the [remove_object] tag?

[cooljeanius]

this would be a lot easier if I could select the text from the inspector and copy it and paste it here... maybe I'll open a separate issue against upstream Wesnoth about that...

(this is now wesnoth/wesnoth#8032 btw)

[cooljeanius]

ok so actually upon further inspection, the issue of the overlays is actually already solved, it's just the part where the death of a ship that previously carried a hero can cause defeat (even after having unloaded that hero) that remains...

[cooljeanius]

...oh, and one other thing that I'd like to slot under this issue (although it shouldn't be considered a blocker for 1.0 like the linked deaths part is), is that when a ship can unload a passenger to multiple hexes, the player should be able to choose the hex, rather than the game just choosing one for them.