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

Visibility ValueRefs #1764

Merged
merged 19 commits into from Sep 29, 2017

Conversation

Projects
None yet
3 participants
@geoffthemedio
Copy link
Member

commented Sep 23, 2017

Adds a Visibility ValueRef type, including an EmpireObjectVisibility lookup. Includes parsing. Also some related Dump function tweaks.

@geoffthemedio

This comment has been minimized.

Copy link
Member Author

commented Sep 25, 2017

One potential issue is that order of effects execution may be partly lost, in that only the last-executed effect will be remembered, which sets which ValueRef to evaluate later to get the final visibility for the object. Will that be a problem in practice?

@geoffthemedio geoffthemedio force-pushed the VisibilityValueRef branch from c9aecdf to 7477860 Sep 26, 2017

@Dilvish-fo

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

...only the last-executed effect will be remembered...Will that be a problem in practice?

It seems to me it could be a problem if an effect granting max(Partial, Value) were being overwritten by one granting just max(Basic, Value), and I think such situations could be encountered with current content. Instead of having EmpireObjectVisValueRefMap record a single entry for each target object, how about having it keep a vector of entries for each target object?

(edited to correct which visibilities map I was referring to)

@geoffthemedio

This comment has been minimized.

Copy link
Member Author

commented Sep 26, 2017

Just adding a vector of ValueRef to evaluate in order for each object would be OK, and would automatically keep track of the order (and thus effect priorities) of effects applied to each object. But is it expected that there would be cases where the Visibility would be determined using a statistic, which needs to be evaluated at a particular time with respect to other effects setting visibility? This whole system already complicates things a bit with doing that, as the gamestate at the time the effect is run and the time when the visibilities are evaluated will be potentially different, but I think that just has to be accepted... But it could be set up to to run through all the saved ValueRefs in a particular oder, to ensure that effect priorities are respected between different objects, not just per-object. But perhaps that's unnecessary complication for now?

@Dilvish-fo

This comment has been minimized.

Copy link
Member

commented Sep 26, 2017

But is it expected that there would be cases where the Visibility would be determined using a statistic, which needs to be evaluated at a particular time with respect to other effects setting visibility?

Well, it's not something that I myself expect. I do expect that it would be sufficient to simply note these kinds of constraints/limitations/complications in the SetVisibility subsection I just added to the FOCS Scripting Details wiki page. For now I also added a link back to here. Please double check it to see I got the new parsing description right.

But perhaps that's unnecessary complication for now?

Seems so to me.

geoffthemedio added some commits Sep 1, 2017

-Added Visibility ValueRef template types.
-Made SetVisibility store a ValueRef rather than an already-determined Visibility.
-Removed / commented out serialization of effect-determined visibilities.
-Partial parsing for Visibility ValueRefs
-grooming
-grooming
-explanitory comment
Swapped order of empire and visibility parameters in SetVisibility ef…
…fect parsing, because a visibility ValueRef could itself have an empire parameter and this order will hopefully be clearer.
Made ComplexVariable<T>::Dump() call Variable<T>::Dump() to get the p…
…roperty name instead of generic "ComplexVariable" as the output.
-Added free variable parser to enum parsers.
-For Visibility enum parser, added (current/initial) Value parser.
Made telepathic detection and scrying sphere set visibility to the gr…
…eater of current and basic/partial, respectively.

@geoffthemedio geoffthemedio force-pushed the VisibilityValueRef branch from 7477860 to 5f36de6 Sep 27, 2017

@geoffthemedio

This comment has been minimized.

Copy link
Member Author

commented Sep 27, 2017

Storing and evaluating the vector of ValueRefs for visibility should be implemented now. Quick test with some simple visibility-granting effectsgroups on humans seems to work.

@geoffthemedio geoffthemedio merged commit 02fb75f into master Sep 29, 2017

4 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@geoffthemedio geoffthemedio deleted the VisibilityValueRef branch Oct 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.