Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[object] [effect] [filter] status= doesn't work #7400

Closed
Toranks opened this issue Feb 20, 2023 · 10 comments
Closed

[object] [effect] [filter] status= doesn't work #7400

Toranks opened this issue Feb 20, 2023 · 10 comments
Labels
Bug Issues involving unexpected behavior. WML Issues involving the WML engine or WML APIs.

Comments

@Toranks
Copy link
Contributor

Toranks commented Feb 20, 2023

Game and System Information

1.17.13
Windows 7

Description of the bug

Apparently other status like poisoned are correctly detected, but this is not the case with uncovered, which prevents creating [effects] based on the uncovered status.
NEW NOTE: Doesn't work with some non-persistent states, as status, HP, movement points, etc.

Steps to reproduce the behavior

Try adding this to any unit and do an attack, the unit never shows the icon.

			[modifications]
				[object]
					id=uncovered_icon
					[effect]
						add="misc/loyal-icon.png"
						apply_to=overlay
						[filter]
							status=uncovered
						[/filter]
					[/effect]
				[/object]
			[/modifications]

Change that with status=poisoned on a poisoned unit another StandardUnitFilter and see how the icon appears.

Expected behavior

When the unit becomes uncovered, the icon should update

Additional context

  • status=uncovered as StandardUnitFilter works on [modify_unit] [filter], [event] [filter] and other places, the problem is only inside [effect]

  • It is not a new bug. I remember having issues with the uncovered status and using objects in the past, but couldn't reproduce it correctly until now

NEW NOTE: as said in the comments, this behavior is not expected, therefore I close the issue

@Toranks Toranks added the Bug Issues involving unexpected behavior. label Feb 20, 2023
@Wedge009 Wedge009 added the WML Issues involving the WML engine or WML APIs. label Feb 21, 2023
@stevecotton
Copy link
Contributor

How is the "do an attack" related to the rest of this? I'm not understanding how attacking is involved with the status.

@Toranks
Copy link
Contributor Author

Toranks commented Apr 11, 2023

To give uncovered automatically:
uncovered: if 'yes', the unit has performed an action (e.g. attacking) that causes it to no longer be hidden until the next turn.

@CelticMinstrel
Copy link
Member

CelticMinstrel commented Apr 11, 2023

Your comparison to poison seems to be incorrect. You're adding the poison overlay modification to a poisoned unit, but you're adding the uncovered overlay modification to a non-uncovered unit, assuming I read your report properly.

If you add the uncovered modification after the unit attacks, does the overlay appear? Have you tried adding the poisoned modification to a non-poisoned unit and then poisoning it?

@Toranks
Copy link
Contributor Author

Toranks commented Apr 11, 2023

No, what I said is a way to reproduce it, but the icon changes in real time after a unit that was not previously poisoned is poisoned. Any attack updates the status of this icon, checked with other status, and other different properties, and also with ability_type_active. I've been using this methods for a long time, and I've discovered that status=uncovered is the ONLY thing that doesn't works on [effect][filter], AFAIK.

So no to your first question, the overlay never appears, no matter what I do (attack, move, next turn, giving status manually, etc...). Yes to your second question, with not only poisoned, but many other examples, those mentioned above, other StandardUnitFilters, StandardTerrainFilters, and detection of adjacent enemies, with perfect behaviour since @stevecotton fixed this crash:
#7238

@CelticMinstrel
Copy link
Member

the icon changes in real time after a unit that was not previously poisoned is poisoned.

Just making sure, but… this includes in the middle of an attack, for example?

@gfgtdf
Copy link
Contributor

gfgtdf commented Apr 12, 2023

I need to say here that while it might work for you, (especially when you often rebuild the units in wml code) using filters in [effect] to filter based on 'nonpersistent' stats like hp/movesleft/statuses etc can in theory cause OOS (not here since its only used for overlays which have no impact on the gamestate), since reloading the game will invoke a rebuild, which might then change the units stats.

@CelticMinstrel
Copy link
Member

And generally speaking it is not expected to work. I don't even know what could be making it work, but it might even be a bug that it works for poison.

@Toranks
Copy link
Contributor Author

Toranks commented Apr 12, 2023

@CelticMinstrel @gfgtdf I think you are right, since I have tried with hitpoints, movements, and the same thing happens to me. And it also happens to me again with other status using apply_to=overlay. I think what @gfgtdf says is probably correct and it used to work for me before because I used other separate unit rebuild with WML as an extra check method. Sorry for giving a false clue. So it's not supposed to work with nonpersistent stats? ability_type_active is considered persistent or not persistent? Because ability_type_active works with overlay. And this can do a potential OOS?

2023-03-28_20-05-54.mp4

@gfgtdf
Copy link
Contributor

gfgtdf commented Apr 12, 2023

And this can do a potential OOS?

Like i said, if your effect only changes the overlay, it won't cause OOS. But when you for example have code like

[effect]
	apply_to=new_ability
	 .. some ability code
	[filter]
		status=poisoned
	[/filter]
[/effect]

The the unit might for example have the object when the object is applied for the first time. And then when the unit loses their poisoned state the unit still has the ability since the unit was not rebuilt (unless you rebuild it manually via wml). Then when the game is reloaded which automaticially rebuilds the unit it might lose the ability. (In reality this only happens in rather rare cases, but its still possible afaik)

@CelticMinstrel
Copy link
Member

I think ability_type_active and ability_id_active should be considered non-persistent, yes. I wouldn't expect them to work reliably on a modification.

@Toranks Toranks changed the title [object] [effect] [filter] status=uncovered doesn't work [object] [effect] [filter] status= doesn't work Apr 12, 2023
@Toranks Toranks closed this as not planned Won't fix, can't repro, duplicate, stale Apr 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Issues involving unexpected behavior. WML Issues involving the WML engine or WML APIs.
Projects
None yet
Development

No branches or pull requests

5 participants