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

Extension variables #6557

Merged
merged 15 commits into from May 14, 2024
Merged

Extension variables #6557

merged 15 commits into from May 14, 2024

Conversation

D8H
Copy link
Collaborator

@D8H D8H commented May 9, 2024

Changes

  • Handle unified global, scene and local variables in extensions.
  • New variable instructions and expressions can't access scene events.
  • Legacy instructions and expressions still allow to access scene events variables.
  • Fix ArbitraryEventsWorkerWithContext inheritance to avoid bad segment crashes.
  • Fix free function events not being stripped out of data.json

@D8H D8H force-pushed the unify-variable-extension branch from e6a1aed to 1258a76 Compare May 10, 2024 09:24
@D8H D8H force-pushed the unify-variable-extension branch from 1258a76 to 623ddcf Compare May 10, 2024 14:22
@D8H D8H marked this pull request as ready for review May 10, 2024 15:19
@D8H D8H requested a review from 4ian as a code owner May 10, 2024 15:19
Local
Local,
ExtensionGlobal,
Extension
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, maybe

Suggested change
Extension
ExtensionScene

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(someone could say that there is no reason "Extension" would mean something tied to the scene, the default "Extension" could as well mean "global variables" that live forever because the extension lives forever when the game runs)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree but I used a different enum value because I was afraid of a confusion in the generator. Maybe we can try to refactor later?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean a confusion if you rename Extension to ExtensionScene?

That should be fairly safe now, the compiler will let you know all the changes to do?

Copy link
Collaborator Author

@D8H D8H May 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I meant using existing Scene and Global + the scope as there is no "any" variable that can be both Global and ExtensionGlobal but I'm not sure if it would be better or not (or even feasible).

@@ -39,6 +44,12 @@ export default function OptionsEditorDialog({
title={<Trans>{eventsFunctionsExtension.getName()} options</Trans>}
secondaryActions={[
<HelpButton key="help" helpPagePath="/extensions/create" />,
<RaisedButton
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for now but we really need to move it to a tree item soon then! :)

GDJS/Runtime/runtimegame.ts Outdated Show resolved Hide resolved
@D8H D8H merged commit 2758697 into unify-variable May 14, 2024
4 of 6 checks passed
@D8H D8H deleted the unify-variable-extension branch May 14, 2024 17:33
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

2 participants