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

call renpy.const for stores marked with _constant = True #5531

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Elckarow
Copy link
Contributor

since constant stores aren't supposed to change, let's use that for screen optimization

@renpytom
Copy link
Member

I'm not convinced that this is correct, and I need to think about it a bit more. (It's quite possible that _constant is misnamed, and it's more propertly _do_not_rollback.) This may be fixable, but I want to think about this before committing to this change.

@mal
Copy link
Member

mal commented May 22, 2024

At least as I understand it, calling renpy.const with the module names wouldn't be beneficial anyway as the whole name (as it would appear in screens) needs to be declared. i.e. renpy.const('renpy.foo') isn't going to result in renpy.foo.bar being treated as const.

edit: TIL. For future readers, see the post below.

@Elckarow
Copy link
Contributor Author

Elckarow commented May 22, 2024

renpy.const('renpy.foo') isn't going to result in renpy.foo.bar being treated as const.

it does treat it as const.

image

@Gouvernathor
Copy link
Member

About what Tom said : it's currently documented as being related to constantness, and we offer no guarantee about rolling back. In particular, the annoying question of an object accessed both through define and default, would rise again for an object present in that store but also reachable from a rollbackable object. It would rise if we start documenting it that way.
I think we should expand on the way to constantness.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants