endless-sky icon indicating copy to clipboard operation
endless-sky copied to clipboard

feat(enhancement): Dynamic condition access

Open tibetiroka opened this issue 1 year ago • 5 comments

Feature

This PR adds support for condition expansions inside conditions.

Summary

With this PR, you can now use $[] to expand a condition's value inside a condition name. This lets you get/set conditions without having to write the exact condition name itself, which was previously required. This gives us proper dynamic random access to the 'memory' of a save file.

Examples

If "counter" = 5, then

"value $[counter]" = 10 is the same as "value 5" = 10.

Expansions can even be nested, if necessary.

Usage examples

This could be used in any scenario where e.g. counting is required; maybe you want to show a choice in a loop and remember which option the player chose each time. This is now possible. This also lets one save independent data for each instance of a repeatable mission.

This also greatly simplifies scripting in general; although the ES data syntax was already Turing-complete, our lack of dynamic random memory access meant the size of usable memory was limited by the size of the program itself. For instance, a Brainfuck Interpreter can already be written in the ES data language, but is not practical. This PR changes that.

Testing Done

Unit tests.

Performance Impact

Fairly small, though this does add an additional string-copy for any condition names. I'd expect the impact to be negligible.

tibetiroka avatar Feb 14 '24 18:02 tibetiroka

For instance, a Brainfuck Interpreter can already be written in the ES data language, but is not practical. This PR changes that.

~Why though?~

RisingLeaf avatar Feb 15 '24 11:02 RisingLeaf

For instance, a Brainfuck Interpreter can already be written in the ES data language, but is not practical. This PR changes that.

~Why though?~

Which part? :)

tibetiroka avatar Feb 15 '24 13:02 tibetiroka

Any sufficiently complicated C or Fortran program contains an ad hoc, informally-specified, bug-ridden, slow implementation of half of Common Lisp.

~~One more step towards Greenspun's tenth rule :)~~

Koranir avatar Feb 16 '24 01:02 Koranir

Isn't it more common to use ${} instead of $[] for variable substitutions?

Koranir avatar Feb 16 '24 01:02 Koranir

Well, we already have <> for substitutions, ${} for phrases and &[] for formatted conditions...

tibetiroka avatar Feb 16 '24 04:02 tibetiroka

This could be used in any scenario where e.g. counting is required; maybe you want to show a choice in a loop and remember which option the player chose each time. This is now possible. This also lets one save independent data for each instance of a repeatable mission.

Can't you do that by manually creating the conditions, which you presumably need to do anyways to test them later on? Idk it seems like the use case for this is fairly contrived.

quyykk avatar Mar 11 '24 21:03 quyykk

which you presumably need to do anyways to test them later on?

The point of this PR is that you do not have to create them for testing, you only use a 'template' of sorts. The save file will have those conditions, sure, but you don't need to reference all of them manually.

tibetiroka avatar Mar 12 '24 06:03 tibetiroka

This is a really cool feature, and I'm very impressed that you took the time to actually write a BrainFuck interpreter in the ES data syntax.

But I'm also very concerned about the added complexity, both in code;

  • A single condition-name no longer refers to a single condition; which makes it more difficult to:
    • cache conditions (if we ever want to do so for performance reasons, for example when we also start using conditions during realtime/flight)
    • optimize conditionsets (Tehhowch suggested something like this quite some time ago)
    • speed up conditions by interning the strings in conditionStores and in ConditionSets
  • The additional string lookups will cost some performance, on every condition-access. (Mostly relevant if we want to use conditions during realtime/flight.)

As well as the complexity in the concepts our story-writers need to deal with;

  • Would story-writers be able to use this new syntax? (It probably looks simple to you, but I have seen programming experts that were very confused by Reflective Programming.
  • Would story-reviewers be able to easily review stories that were written using this syntax?
  • Is there really a use-case for remembering independent data for repeatable missions? (I expect multiple non-repeating missions to usually prevent such a need.)
  • Do story-writers have a strong need for the ability to remember the various choices in loops?

The main goal of the ES code is to be a storytelling engine together with 2D space flying/trading/fighting game. Features like this might actually scare away some story-writers; the syntax we currently use is already quite complex and we should aim to keep the syntax simple, since we really like new game-content. We also really like features added that help speed-up the game or that significantly add new story-building-tools or eye-candy.

Sorry, I feel that this is a very cool feature, but that the benefits do not outweigh the added complexity.

petervdmeer avatar Mar 18 '24 20:03 petervdmeer

But I'm also very concerned about the added complexity, both in code;

* A single condition-name no longer refers to a single condition; which makes it more difficult to:
  
  * cache conditions (if we ever want to do so for performance reasons, for example when we also start using conditions during realtime/flight)
  * optimize conditionsets (Tehhowch suggested something like this quite some time ago)
  * speed up conditions by [interning the strings in conditionStores and in ConditionSets](https://github.com/endless-sky/endless-sky/pull/5748)

This does make optimization more difficult. String interning should still be doable (hopefully), especially if there is a new non-expanded getter added to the store. I have doubts about blocking a PR because of some other potential future work that might or might not happen (only one of these have a PR, and that seems to only be updated once a year).

* The additional string lookups will cost some performance, on every condition-access. (Mostly relevant if we want to use conditions during realtime/flight.)

Yeah, that's true... It's something we would need to benchmark first. We could always say something like 'only conversations support this feature', it wouldn't be the first time.

As well as the complexity in the concepts our story-writers need to deal with;

* Would story-writers be able to use this new syntax? (It probably looks simple to you, but I have seen programming experts that were very confused by [Reflective Programming](https://en.wikipedia.org/wiki/Reflective_programming).

* Would story-reviewers be able to easily review stories that were written using this syntax?

They don't have to use it. It would probably make the wiki tutorials more complicated, though. Reflection was never confusing to me either, so I may not be the best person to ask.

* Is there really a use-case for remembering independent data for repeatable missions? (I expect multiple non-repeating missions to usually prevent such a need.)

* Do story-writers have a strong need for the ability to remember the various choices in loops?

Honestly, I don't really think so. The programming opportunities are great, however.

The main goal of the ES code is to be a storytelling engine together with 2D space flying/trading/fighting game. Features like this might actually scare away some story-writers; the syntax we currently use is already quite complex and we should aim to keep the syntax simple, since we really like new game-content. We also really like features added that help speed-up the game or that significantly add new story-building-tools or eye-candy.

Can we call it a feature for better supporting total conversion plugins? ;)

I realize this is not going to see much use in vanilla content. It's definitely a nice thing to have for some of my personal projects, but it's not a necessity for ES as such.

tibetiroka avatar Mar 18 '24 21:03 tibetiroka

I have a question, so correct me if I am wrong but the main use case of this would be to simulate an array, right? Or create loops.

RisingLeaf avatar Mar 18 '24 21:03 RisingLeaf

Honestly, I don't really think so. The programming opportunities are great, however.

Do we really want to turn the data files into an actual programming language? Might as well use a real one like Lua at that point ^^. just my 2c

quyykk avatar Mar 18 '24 21:03 quyykk

Do we really want to turn the data files into an actual programming language?

Preferably not, and not for the sake of other programming. There already has been a no'd Lua PR. Data files should remain more simpler and easy for creators to manipulate - I realize there's such a thing as entry-level coding and learning that, but the last thing we should do is make everything in ES's files hard code.

(I could be interpreting the comment wrong and you're suggesting similar though, by telling people to go code otherwise. :p)

Saugia avatar Mar 18 '24 21:03 Saugia

I have a question, so correct me if I am wrong but the main use case of this would be to simulate an array, right?

Yes, that is what it could be used for.

Or create loops.

No, this doesn't add loops. But loops can already be done by using branching with jump labels.

petervdmeer avatar Mar 18 '24 22:03 petervdmeer

My main point is that features in the codebase are not "available for free" after they get merged. When we support this feature today, then we also need to support it tomorrow. Which means that when we refactor code or add other new features near the ConditionSets or ConditionStores, then they need to be compatible with features already existing in the code (including the feature from this PR, if we choose to merge it). Conditions are quite central and quite critical to the game, and we did get quite some significant development on them in the last few years, so it seems reasonable to expect also significant development focus on conditions-code in the future (better and more than the 3 poor examples I came up with).

I still consider the future costs of merging this feature (by making other work on conditions more difficult, and thus less likely to happen) to be higher than the benefits of merging this feature.

petervdmeer avatar Mar 18 '24 23:03 petervdmeer