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

stateSchema bug validating between value vs composite schemas #247

Open
zepumph opened this issue Sep 17, 2021 · 3 comments
Open

stateSchema bug validating between value vs composite schemas #247

zepumph opened this issue Sep 17, 2021 · 3 comments

Comments

@zepumph
Copy link
Member

zepumph commented Sep 17, 2021

Related to work in #245,

@jessegreenberg and I found a bug where because a stateschema was given a composite schema (Vector2IO) incorrectly, and should have been given a value stateSchema (NullableIO( Vector2IO )), when passing in a value (null), we got a truly atrocious error message:

Error: TypeError: Cannot read properties of null (reading 'hasOwnProperty')
    at eval (eval at <anonymous> (StateSchema.js:169), <anonymous>:1:14)
    at StateSchema.js:169
    at Array.forEach (<anonymous>)
    at checkLevel (StateSchema.js:167)
    at StateSchema.checkStateObjectValid (StateSchema.js:179)
    at IOType.isStateObjectValid (IOType.js:334)
    at IOType.validateStateObject (IOType.js:376)
    at StateSchema.js:174
    at Array.forEach (<anonymous>)
    at checkLevel (StateSchema.js:167)

Basically, when you get down to it, checkLevel expects a composite stateObject (an object), but it was null, a valid value for NullableIO(Vector2IO).

We can use SceneryEventIO to achieve this in the mirror inputs wrapper when you select a menu item with keyboard navigation.

The purpose of this issue is to make a better assertion earlier in the chain.

@zepumph zepumph self-assigned this Sep 17, 2021
@zepumph zepumph removed their assignment Mar 3, 2023
@samreid samreid self-assigned this Mar 27, 2023
@samreid
Copy link
Member

samreid commented Mar 27, 2023

The mirror inputs wrapper isn't working at the moment. Something about the resizeAction getting [x,y] instead of {x,y}? I wonder if @zepumph and I will visit this issue when we work on #282?

@samreid
Copy link
Member

samreid commented Apr 6, 2023

I did not have time to address this issue during this iteration, and am not planning to work on it soon. Unassigning for now.

@samreid samreid removed their assignment Apr 6, 2023
@zepumph zepumph removed their assignment Apr 6, 2023
@zepumph
Copy link
Member Author

zepumph commented Apr 6, 2023

If we encounter this bug during serlization again, that will help raise this issue's priority. We will more likely not encounter it in the same way as when originally reported (keyboard nav + input event recording + specific case).

@zepumph zepumph self-assigned this Jun 23, 2023
@zepumph zepumph removed their assignment Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants