-
Notifications
You must be signed in to change notification settings - Fork 737
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
Add Deprecate components support #3795
Conversation
//If not enabled, then do not add CanInteractWith Condition or event handlers: | ||
if (!GVAR(enable)) exitWith {}; | ||
if (!GVAR(enable) || GVAR(isEnabled)) exitWith {}; | ||
GVAR(isEnabled) = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is that for again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ensures that the module will not load in twice
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How could it? Unless someone put 2 of those down (in which case he should get all the RPT warnings).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are quite right - this isn't necessary here, but it is for the port to ACEX. It should not have been commited with this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How are we going to handle that for components which have modules that can be placed multiple times (eg. slideshow)? Current isEnabled check in ACEX sitting component would prevent that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same way we handle functions that we deprecate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Never mind, slideshow doesn't even have a XEH postInit, was thinking of something else.
IsEnabled check is not necessary here. It should not have been included, as it was a left over from testing for migrating sitting to ACEX.
@Glowbal Will this print warnings for sitting? If so it has to be taken out before release. |
When merged this pull request will: