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

fix system ship unstealthiness #3839

Merged

Conversation

agrrr3
Copy link
Contributor

@agrrr3 agrrr3 commented May 17, 2022

lienrag found the fleet unstealthiness to misbehave if in transit. (e.g. screenshot in forum )

if a candidate is not in a system (has SystemID -1) it matches nothing. the -1 as target is used to signal "match any system" (i.e. an SystemID but -1).

not sure if the implementation of InSystem is a bug or if it is correct and simply not helpful/not intuitive. Not Not InSystem id = xxx probably has to be the same as InSystem id = xxx for all xxx .

so for fleet unstealthiness code. the ship the stealth effect applies to is the target. the ships which are counted are the candidates.
according to my code dive the stealth of a ship on a starlane would be decreased depending on the count of all ships which are not on a starlane - which is clearly a bug.

the scope condition needs an InSystem condition (without id).

this PR contains three commits; one fixes a typo; one fixes the unstealthiness ; one moves the effect to a more fitting focs file

@geoffthemedio geoffthemedio added the category:bug The Issue/PR describes or solves a perceived malfunction within the game. label May 18, 2022
@geoffthemedio
Copy link
Member

Not Not InSystem id = xxx probably has to be the same as InSystem id = xxx for all xxx

Not sure what your point is...

@geoffthemedio geoffthemedio merged commit bed41d1 into freeorion:master May 18, 2022
@geoffthemedio geoffthemedio added component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. category:tweak The PR contains insignificant code changes, like code style grooming or value tweaking. status:merged All relevant commits of this PR were merged into the master development branch. labels May 18, 2022
@agrrr3
Copy link
Contributor Author

agrrr3 commented May 19, 2022

Not Not InSystem id = xxx probably has to be the same as InSystem id = xxx for all xxx

Not sure what your point is...

probably my point was i am not sure that the InOrIsSystem::Eval works correctly for the both search domains. but probably it does. and changing it (see below) would also be fine.

@geoffthemedio the underlying question was if we should change the behaviour of the condition not to interpret -1 as "match any system"; probably by using a different int constant (e.g. ANY_SYSTEM_ID -2)

@geoffthemedio
Copy link
Member

probably my point was i am not sure that the InOrIsSystem::Eval works correctly for the both search domains.

What are the two search domains you mean?

the underlying question was if we should change the behaviour of the condition not to interpret -1 as "match any system"; probably by using a different int constant (e.g. ANY_SYSTEM_ID -2)

This is how most / all conditions and valuerefs work. ID -1 means "no particular object", and may result in matching anything or nothing, depending on the situation. Specifying or using a valueref that evaluates to -1 is also often different from specifying nothing, and sometimes one needs to include a no parameter version of a condition or apply earlier filtering to be sure one is getting a valid object ID and not -1 before passing it on...

@Vezzra Vezzra added this to the v0.5 milestone May 20, 2022
@agrrr3
Copy link
Contributor Author

agrrr3 commented May 23, 2022

probably my point was i am not sure that the InOrIsSystem::Eval works correctly for the both search domains.

What are the two search domains you mean?

matches and non_matches

the underlying question was if we should change the behaviour of the condition not to interpret -1 as "match any system"; probably by using a different int constant (e.g. ANY_SYSTEM_ID -2)

This is how most / all conditions and valuerefs work. ID -1 means "no particular object", and may result in matching anything or nothing, depending on the situation. Specifying or using a valueref that evaluates to -1 is also often different from specifying nothing, and sometimes one needs to include a no parameter version of a condition or apply earlier filtering to be sure one is getting a valid object ID and not -1 before passing it on...

ok, so InSystem id = Target.SystemID matching any object on a starlane is expected behavior. I added documentation entries in the wiki. case closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bug The Issue/PR describes or solves a perceived malfunction within the game. category:tweak The PR contains insignificant code changes, like code style grooming or value tweaking. component:content scripting The Issue/PR deals with the FOCS language, turn events or the universe generator. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants