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

FIX: InputActionReference when using FEPM (ISX-1968) #1949

Merged
merged 1 commit into from
Jun 20, 2024

Conversation

timkeo
Copy link
Collaborator

@timkeo timkeo commented Jun 18, 2024

Description

When Domain Reloads are disabled, InputActionReference instances continue to reference the "old" InputAction object from the previous PlayMode session. This fix clears the InputAction reference when exiting PlayMode allowing it to be reloaded in the next session.

Although Configurable PlayMode options, aka Fast Enter PlayMode, isn't currently supported, a number of tickets have been filed against this issue including:

indicating FEPM is quite a popular feature and fixing this issue is beneficial to our customers.

Furthermore, the underlying problem is a bug in its own right regardless of FEPM: the underlying Action instance is invalid for EditMode and holding the reference is dangerous as it keeps the object "alive" along with the pointers to native memory buffers that are de-allocated when exiting PlayMode. In short, attempting to access the ActionReference after leaving PlayMode can cause an AV and crash the Editor.

Changes made

When exiting PlayMode, all InputActionReference instances are queried and their m_Action field is reset. This forces a "reload" of the Action from the Asset the next time its accessed just as if a Domain Reload had occured.

Notes

This issue is completely separate from my CoreCLR refactor and can be fixed independently of it.

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@timkeo timkeo force-pushed the isx-1968-fix-inputactionreference branch from d5185f6 to 2b66a32 Compare June 18, 2024 23:20
@timkeo timkeo marked this pull request as ready for review June 18, 2024 23:21
@timkeo timkeo requested review from ekcoh and Pauliusd01 June 18, 2024 23:21
When Domain Reloads are disabled, InputActionReference instances continue to reference the "old" InputAction object from the previous PlayMode session.

This fix clears the InputAction reference when exiting PlayMode allowing it to be reloaded in the next session.
@timkeo timkeo force-pushed the isx-1968-fix-inputactionreference branch from 2b66a32 to 7fafc88 Compare June 18, 2024 23:46
Copy link
Collaborator

@Pauliusd01 Pauliusd01 left a comment

Choose a reason for hiding this comment

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

LTGM, made sure that the mentioned bugs are fixed, ran through a few testing scenes with the domain reload disabled, including ones that save things such as rebinding menus. Did not see any issues.

Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

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

Excellent PR description and great to get this fixed. LGTM

@timkeo
Copy link
Collaborator Author

timkeo commented Jun 20, 2024

The tvOS failure is a per-existing issue and unrelated to the PR, which is an Editor fix that doesn't affect Players.

@timkeo timkeo merged commit 6037d4d into develop Jun 20, 2024
78 of 80 checks passed
@timkeo timkeo deleted the isx-1968-fix-inputactionreference branch June 20, 2024 15:38
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.

3 participants