[ATL] ATL blocks with string interpolation via `show expression MyDisplayable(text="[my_var]")` and dynamic variables in labels (`linear my_duration`) don't play nice with ATL deep comparison

[Rastagong]

Hi,

This is a very much a very rare edge case...
I'm not sure a fix would be worth it at all depending on complexity/your design preferences, but still wanted to document it for anyone else, just in case!

---

Context

ATL deep comparison was introduced in Ren'Py 8.1.2 to determine more thoroughly if an ATL transform has changed or not. See commits 468733b but also especially 9c7b8a6. Previously, ATL comparison was only performed via loaded variables comparisons (cf. b2a49cc for the removed code of loaded variables comparisons via find_loaded_variables).

It works, but when porting an old project to recent Ren'Py versions, I found an edge case where string interpolation, when used in a show expression statement, combined with an ATL interpolation that references a dynamic label variable, will make deep comparison fail, even though the code is valid Ren'Py script.

---

Minimal example to reproduce the anomaly

To be tested with any version starting from Ren'Py 8.1.2, which introduces ATL deep comparison. No anomaly with Ren'Py <= 8.1.1.

label display_date_somewhere(new_date, fade_duration=10.0):
    $ interpolated_text = Text("[new_date]")
    $ my_window = Window(child=interpolated_text, background='#000', padding=(10, 10), style="default")
    show expression my_window onlayer screens:
        alpha 0.0
        linear fade_duration alpha 1.0
    return

label start:
    "Let's start it."
    call display_date_somewhere("June 18")

    pause 1.0
    
    show bg uni:
        linear 2.0 zoom 2.0
    with dissolve

Traceback (line numbers within Ren'Py core may be off; I debugged directly in the files with statements of mine):

Compiling ATL code at game/script.rpy:23
  File "game/script.rpy", line 34, in script
    with dissolve
  File "game/script.rpy", line 23, in <module>
    linear fade_duration alpha 1.0
            ^^^^^^^^^^^^^         
NameError: name 'fade_duration' is not defined

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "game/script.rpy", line 34, in script
    with dissolve
  File "game/script.rpy", line 34, in script
    with dissolve
  File "game/script.rpy", line 23, in <module>
    linear fade_duration alpha 1.0
            ^^^^^^^^^^^^^         
NameError: name 'fade_duration' is not defined

-- Full Traceback ------------------------------------------------------------

Traceback (most recent call last):
  File "game/script.rpy", line 34, in script
    with dissolve
  File "renpy/ast.py", line 1591, in execute
    renpy.exports.with_statement(trans, paired=paired)
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "renpy/exports/statementexports.py", line 260, in with_statement
    return renpy.game.interface.do_with(trans, paired, clear=clear)
           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "renpy/display/core.py", line 1579, in do_with
    return self.interact(
           ~~~~~~~~~~~~~^
        trans_pause=True, suppress_overlay=not renpy.config.overlay_during_with, mouse="with", clear=clear
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "renpy/display/core.py", line 2219, in interact
    repeat, rv = self.interact_core(
                 ~~~~~~~~~~~~~~~~~~^
        preloads=preloads,
        ^^^^^^^^^^^^^^^^^^
    ...<4 lines>...
        **kwargs,
        ^^^^^^^^^
    )  # type: ignore
    ^                
  File "renpy/display/core.py", line 2473, in interact_core
    self.transition_from[k] = self.old_scene[k]._in_current_store()
                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "renpy/display/layout.py", line 736, in _in_current_store
    new_d = old_d._in_current_store()
            ~~~~~~~~~~~~~~~~~~~~~~~^^
  File "renpy/display/layout.py", line 709, in _in_current_store
    d = new_sle.displayable._in_current_store()
        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "renpy/display/transform.py", line 1234, in _in_current_store
    rv.take_execution_state(self)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "renpy/atl.py", line 569, in take_execution_state
    block = self.compile()
            ~~~~~~~~~~~~^^
  File "renpy/atl.py", line 757, in compile
    block = self.atl.compile(self.context)
            ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "renpy/atl.py", line 951, in compile
    statements = [i.compile(ctx) for i in self.statements]
                  ~~~~~~~~~^^^^^                          
  File "renpy/atl.py", line 1325, in compile
    duration = ctx.eval(self.duration)
               ~~~~~~~~^^^^^^^^^^^^^^^
  File "renpy/atl.py", line 403, in eval
    return renpy.python.py_eval(expr, locals=self.context)
           ~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "renpy/python.py", line 1331, in py_eval
    return py_eval_bytecode(code, globals, locals)
           ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "renpy/python.py", line 1320, in py_eval_bytecode
    value = eval(bytecode, globals, locals)
            ~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "game/script.rpy", line 23, in <module>
    linear fade_duration alpha 1.0
            ^^^^^^^^^^^^^         
NameError: name 'fade_duration' is not defined

---

The anomaly immediately disappears if you:

• Use a Text element without string interpolation likeText(new_date), quite simply, which is really the thing to do here!
• Or if you do away with the dynamic label variable in the ATL block: linear 1.0
• Or, better still, if you move over the linear fade_duration to a transform to isolate it properly, like this:

transform my_transform(fade_duration):
        alpha 0.0
        linear fade_duration alpha 1.0
    
label display_date_somewhere(new_date, fade_duration=10.0):
    $ interpolated_text = Text("[new_date]")
    $ my_window = Window(child=interpolated_text, background='#000', padding=(10, 10), style="default")
    show expression my_window at my_transform(fade_duration) onlayer screens
    return

---

Minimal analysis

From what I understand after a lot of brute-force debugging in the dark:

1. The execution of the display_date_somewhere label works perfectly. The fade_duration dynamic variable is in scope during execution.
2. After the execution of the label, though, when the core needs to check if the ATL has changed for the next transition, the show expression with an interpolated text element is I assume forced to reevaluate that string interpolation.

Found out while debugging that the Window displayable considers that the new Text element child is different from the original one, e.g old is not new here:

    def _in_current_store(self):
        children = []

        changed = False

        for old in self.children:
            new = old._in_current_store() # This is now a different object, I assume because of the string interpolation?
            changed |= old is not new
            children.append(new)

        if not changed:
            return self

As a result, down the stack, the ATLTransform object is forced to take_execution_state() and to be recompiled here, in the context of ATL deep comparison:

        if t.atl.constant != GLOBAL_CONST:
            block = self.get_block()
            if block is None:
                block = self.compile() # That's the line introduced in commit 9c7b8a6

            if not deep_compare(self.block, t.block):
                return

        self.done = t.done
        self.block = t.block
        self.atl_state = t.atl_state
        self.transform_event = t.transform_event
        self.last_transform_event = t.last_transform_event
        self.last_child_transform_event = t.last_child_transform_event

But since we're now outside the scope of the label where the the dynamic fade_duration variable used to exist, it can't be found in the store dictionaries, and bytecode compilation fails with a variable that can't be found.

---

A dirty fix to ignore?

I'm just going to use a proper and clean Transform in my code, I think that's the cleanest solution here!

But otherwise, the following fix in atl.py does seem to work:

        if t.atl.constant != GLOBAL_CONST:
            block = self.get_block()
            #if block is None: I'm removing the compilation prior to deep comparison entirely
            #    block = self.compile() 

            if block is not None and not deep_compare(self.block, t.block): #I perform deep comparison and return early only if the block is already compiled
                return

Deep comparison of ATL blocks is now performed only if the current block is already compiled.
If the block is still uncompiled, we march forward to the self.block = t.block assignment without even performing deep comparison: we just take the value of the new t.block and that's it.
But I'm not sure at all of the implications of such a fix for deep comparison more widely, it seems to sidestep its point entirely. I don't mind at all if this remains unfixable given how strange the conditions to reproduce it are.

[fallbackerik]

I wouldn't be surprised if this code is the source of some non-trivial bugs:

changed = False
# [...]
for old in self.children:
    # [...]
    changed |= old is not new

Can you say easily, what's the difference between the comparison and changed = old is new? Also, it's in a loop, where each step depends on the previous one.

It would probably be better to have something like unchanged = all(old is new for old in self.children) or whatever the intended purpose is.

[renpytom]

I landed what is probably a fix for this. While in the commit itself, I thought it didn't fix this issue (I had been planning on WONTFIXing this), I think it probably does.

That being said, I want to remind you that Ren'Py store variables are dynamically, not statically scoped - and so while the variable should be captured here, that isn't true in all cases.

[Rastagong]

Thank you for taking the time to look into & fix this weird edge case!
Can confirm it now works perfectly on Ren'Py 8.5.0 nightly (8.5.0.25102001+nightly) when it did not on stable (Ren'Py 8.4.1).

And yes, I've been bitten by Ren'Py dynamic scope before, now I try to take it to heart (uniquely named label parameters which thus look/seem lexically scoped, rather than store variables).