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

[Bug]: ArraySchema.unshift() not working as intended. #170

Open
maanex opened this issue Apr 23, 2024 · 4 comments
Open

[Bug]: ArraySchema.unshift() not working as intended. #170

maanex opened this issue Apr 23, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@maanex
Copy link

maanex commented Apr 23, 2024

Bug description

Array.unshift should insert an item at the beginning of the array.
ArraySchema.unshift does the same, if the array has only been .push()'d to so far. As soon as either .pop() or .shift() was called on the ArraySchema, any .unshift() will mess the array up.

Example code below is also available on stackblitz.

import { ArraySchema } from '@colyseus/schema'
const x = new ArraySchema()
x.push(1)
x.push(2)
x.push(3)
x.push(4)
x.push(5)
x.pop()
x.pop()
x.push(9)
console.log(x.toArray()) // "[ 1, 2, 3, 9 ]"
x.unshift(8)
console.log(x.toArray())

// expected
"[ 8, 1, 2, 3, 9 ]"

// actual
"[ 8, 1, 2, 9, 3, 9 ]"

This example uses .pop() though a similar invalid state is reached if .pop() is replaced with .shift()

Optional: Minimal reproduction

https://stackblitz.com/edit/colyseus-schema-issue-report-iwe3ai?file=index.ts

@endel endel added the bug Something isn't working label Apr 23, 2024
@endel
Copy link
Member

endel commented Apr 23, 2024

Thanks for sharing the reproduction scenario, will add this to colyseus/colyseus#641

@maanex
Copy link
Author

maanex commented Apr 23, 2024

Thank you, appreciate your work! Can you recommend a workaround for this until 3.0 is out that doesn't use .unshift() but still inserts an item at the front of the array? My only solution right now is to reverse() push() reverse() - which seems to work fine.

@endel
Copy link
Member

endel commented Apr 23, 2024

As a workaround, you can try flushing the changes (via this.broadcastPatch()) to force the state to sync before doing the .unshift() operation:

x.push(1)
x.push(2)
x.push(3)
x.push(4)
x.push(5)
x.pop()
x.pop()
x.push(9)
this.broadcastPatch(); // HERE
x.unshift(8)

@maanex
Copy link
Author

maanex commented Apr 23, 2024

Will try, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants