SetVisbility issues

This is for directed discussions on immediate questions of game design. Only moderators can create new threads.
Post Reply
Message
Author
User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

SetVisbility issues

#1 Post by Dilvish »

recap: we've just realized that there is a mismatch between how SetVisibility is implemented and one of its uses in the scripts-- it is written up as a hard-setting of visibility that can override most other sources of visibility (albeit with at least one bug and we are having a separate discussion on github to reach consensus on the nature of the bug(s)), but the telepathic detection script (for Trith) uses it for Basic Visibility which would only make sense if it were implemented as a boon-only thing or at least could be overridden by more things than it currently can. All the other current uses of SetVisbility are granting a higher level of visibility and could be reasonably compatible with either significance for SetVisibility.

So I believe we need to make a design decision, whether to either
  1. change the implementation of SetVisibility so it is a boon-only thing (and then the current scripts are fine as-is), or
  2. have SetVisibility be an overriding determination of visibility and add extra Effects, such as GrantVisibility for visibility boons and LimitVisibility for a visibility shroud, and change our current scripts over to using GrantVisibility
Although the first is notably simpler to code-up, I think the latter is straightforward enough also that we don't really need to give weight to that coding difference (I volunteer). Although having three visibility effects is in some ways a greater deal of complexity for a coder to keep in mind, because of their relationship to each other I think that is counterbalanced by the way the set would inherently clarify what each one really means, and of course the flexibility of the extra effects could be nice.

I first raised this question on GitHub, but as a design question I think we should open the discussion up to more people who don't review all the GitHub PRs, hence this post.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: SetVisbility issues

#2 Post by Geoff the Medio »

I don't think the relevant scripts are assuming SetVisibility will only work as a boost. Rather, I think the issue is that effect-derived visibility is applied after the situation in which it was determined no longer exists. The script uses a condition to check the visibility of the target before setting it.

But the effect doesn't set the visibility directly, but rather records what the visibility should be, which is used in a later visibility update. It can't just set the visibility once directly, as that will be overridden by subsequent visibility updates.

But if the effects are run before something like ships moving or invasions, then afterwards a visibility update is run, the conditions used (such as checking if a target is (not) visible to an empire) to decide what visibility to set may no longer exist.

This is, I think with LGM-Doyle suggested clearing the effect-derived visibilities after applying them every time. But I think it's only necessary to clear them after potential changes that may have altered where and at what level visibility should be set.

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

Re: SetVisbility issues

#3 Post by Dilvish »

Geoff the Medio wrote:SetVisibility just setting visibility, rather than having some more-complicated behaviour built in that depends on the current visibility, is or should be sufficient, as long as suitable conditions are used to gate when the visibility is set.
I had considered the possibility of trying to take that approach, but if it is even possible I think it would be tremendously more complicated.

Consider the TELEPATHIC_DETECTION characteristic which led to the combat trouble of issue 1618-- just what conditions would you add to it in order for it to NOT block combat? If I try to consider the various possibilities of fleets moving in and out and or the possible existence of other objects (like planets) owned by the subject empire in the same system, and whether or not there is a scrying sphere or whatnot present-- I really don't see how you account for all that, and it would be tremendously complicated for the scripter to keep track of when they need all that for SetVisibility and when they need some other complicated set of conditions for it, far more so than either having the 3 types of effect or just saying SetVisibility won't decrease visibility.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: SetVisbility issues

#4 Post by Geoff the Medio »

Dilvish wrote:what conditions would you add
I was thinking of just putting the VisibileToEmpire condition in the effect instead of the scope, but that probably won't really help since the actual application of the visibility occurs repeatedly and some time after the effect is executed (to determine what effect-derived-visibilities should be applied for subsequent visibility updates).

Possibly just the visibility-related effects could be re-evaluated and executed each time a visibility update is done, rather than just storing the values of what the visibilities should be set to...

Also, something like max(Target.Visibility, BasicVisibility) could be made possible to script.

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

Re: SetVisbility issues

#5 Post by Dilvish »

Ah, I thought a bit more about what you wrote and now think I understand it better.
But if the effects are run before something like ships moving or invasions, then afterwards a visibility update is run, the conditions used (such as checking if a target is (not) visible to an empire) to decide what visibility to set may no longer exist.
It sounds like you are talking about doing an extra round of assessing effects after the ships have moved (and after invasions and whatnot), or perhaps you are talking about restricting that to only the SetVisibility effect? Or any Effects that influence detection and stealth?

It seems to me that we've already had essentially that same discussion before, about combat being based on meter values from the end of the previous cycle, before movement. If we start saying that the effects applicable to combat *mostly* are evaluated on conditions before movement, except that visibility related effects will get reevaluated *after* movement, well I suppose that could be viable, but it seems to me that it creates decision-making uncertainty that we had previous clearly decided we wanted to avoid.
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

User avatar
Geoff the Medio
Programming, Design, Admin
Posts: 13587
Joined: Wed Oct 08, 2003 1:33 am
Location: Munich

Re: SetVisbility issues

#6 Post by Geoff the Medio »

Dilvish wrote:It sounds like you are talking about doing an extra round of assessing effects after the ships have moved (and after invasions and whatnot), or perhaps you are talking about restricting that to only the SetVisibility effect?
Just SetVisibility, but not even a full effect update for it. The initial idea is to not store a visibility to set, but rather a ValueRef that can be evaluated at the time the visibility is set. This could include things like max or min functions, and refer to the immediate visibility of the target, when determining what visibility level to set.

User avatar
Dilvish
AI Lead and Programmer Emeritus
Posts: 4768
Joined: Sat Sep 22, 2012 6:25 pm

Re: SetVisbility issues

#7 Post by Dilvish »

Geoff the Medio wrote:Just SetVisibility, but not even a full effect update for it. The initial idea is to not store a visibility to set, but rather a ValueRef that can be evaluated at the time the visibility is set. This could include things like max or min functions, and refer to the immediate visibility of the target, when determining what visibility level to set.
Hmm, well if SetVisibility also could work with min and max and the instant Target.Visibility, then it would handle all three cases I was proposing, so that part sounds good to me. Furthermore, I agree that storing such a ValueRef to set rather than the fixed visibility level does sound like it should be able to fix the problem of issue 1618 also. (keep in mind we would also still need to add serialization, to prevent potential discrepancies from a T-1 save).

I am still concerned about this being a deviation from the "know your combat conditions before you send your ships into combat" approach (depending on the scripting details, this approach could easily result in combat or lack thereof that was unexpected by the player, but I suppose there are scenarios that could happen with the other approach also, at least with our current UI that doesn't explicitly try to indicate such direct SetVisibility effects).
If I provided any code, scripts or other content here, it's released under GPL 2.0 and CC-BY-SA 3.0

Post Reply