Skip to content

Type renpy.display.scenelists module #6793

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.

Sign up for GitHub

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

Jump to bottom
Open
arsym-dev wants to merge 1 commit into
base: fix
Choose a base branch
from
Open

Type renpy.display.scenelists module #6793

arsym-dev wants to merge 1 commit into from
+128 −94

Conversation

@arsym-dev
Copy link
Contributor

@arsym-dev arsym-dev commented Dec 10, 2025

Typing and documentation

The only non-typing change is a minor refactor of SceneList.__init__(). The line self.focused = None was common to both branches, so I pulled it out of the branches and annotated it.

The typing highlights logic issues in the module that should be resolved in a future PR.

@renpytom
Copy link
Member

renpytom commented Dec 10, 2025

Please hold off before producing typing PRs like these. These aren't terrible, but I'm considering if we want to move to Python 3.14, and that changes typing.

(I would like to know about the logic issue.)

@arsym-dev
Copy link
Contributor Author

arsym-dev commented Dec 12, 2025

Sure thing, I won't mess with these until we look into python 3.14

Regarding logic, the main issue is that self.config_layer_transform doesn't know if it's dict[str, Transform], or dict[str, list[Transform]].

Acting as list[Transform]

renpy/renpy/display/scenelists.py

Lines 103 to 104 in 2fea804

if i not in self.config_layer_transform:
self.config_layer_transform[i] = []

Acting as Transform

In both snippets, you want to focus on the first and last lines. Notice how old_thing._target() in transform_state() would not work on a list.

renpy/renpy/display/scenelists.py

Lines 763 to 779 in 2fea804

old_transform = self.config_layer_transform.get(layer, None)
new_transform = None
if at_list:
for a in at_list:
if isinstance(a, renpy.display.motion.Transform):
rv = a(child=rv)
else:
rv = a(rv)
rv._unique()
if isinstance(rv, renpy.display.motion.Transform):
new_transform = rv
if new_transform is not None:
self.transform_state(old_transform, new_transform, execution=True)

renpy/renpy/display/scenelists.py

Lines 274 to 288 in 2fea804

def transform_state(self, old_thing, new_thing, execution=False):
"""
If the old thing is a transform, then move the state of that transform
to the new thing.
"""
if old_thing is None:
return new_thing
# Don't bother wrapping screens, as they can't be transformed.
if isinstance(new_thing, renpy.display.screen.ScreenDisplayable):
return new_thing
if renpy.config.take_state_from_target:
old_transform = old_thing._target()

I think the correct behavior is list[Transform] since the comments explicitly state that it's meant to map onto config.layer_transforms. These changes were introduced to address #4562 but I have no idea if it resolved OP's issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@arsym-dev @renpytom