Quenty / NevermoreEngine

ModuleScript loader with reusable and easy unified server-client modules for faster game development on Roblox
https://quenty.github.io/NevermoreEngine/
MIT License
421 stars 125 forks source link

fix: Fix ObserveAtIndex on ObservableSortedList #517

Closed unrooot closed 2 weeks ago

unrooot commented 2 weeks ago

Currently, ObserveAtIndex returns the node instead of the data which breaks a few packages. Switching Rx.start to Rx.map seems to fix the problem and give the desired behavior, but I'm not 100% sure if this is the intended behavior or if this would break anything.

📦 Published PR as canary version: Canary Versions
:sparkles: Test out this PR locally via: ```bash npm install @quenty/adorneeboundingbox@8.12.1-canary.517.2bd4a12.0 npm install @quenty/animationprovider@11.10.1-canary.517.2bd4a12.0 npm install @quenty/chatproviderservice@9.16.1-canary.517.2bd4a12.0 npm install @quenty/cmdrservice@13.14.1-canary.517.2bd4a12.0 npm install @quenty/colorpalette@10.13.1-canary.517.2bd4a12.0 npm install @quenty/gameconfig@12.16.1-canary.517.2bd4a12.0 npm install @quenty/gameproductservice@14.16.1-canary.517.2bd4a12.0 npm install @quenty/highlight@10.13.1-canary.517.2bd4a12.0 npm install @quenty/humanoidspeed@12.14.1-canary.517.2bd4a12.0 npm install @quenty/inputkeymaputils@14.15.1-canary.517.2bd4a12.0 npm install @quenty/observablecollection@12.12.1-canary.517.2bd4a12.0 npm install @quenty/rogue-humanoid@10.14.1-canary.517.2bd4a12.0 npm install @quenty/rogue-properties@11.14.1-canary.517.2bd4a12.0 npm install @quenty/scoredactionservice@16.16.1-canary.517.2bd4a12.0 npm install @quenty/secrets@7.15.1-canary.517.2bd4a12.0 npm install @quenty/settings@11.15.1-canary.517.2bd4a12.0 npm install @quenty/settings-inputkeymap@10.17.1-canary.517.2bd4a12.0 npm install @quenty/soundgroup@1.12.1-canary.517.2bd4a12.0 npm install @quenty/spawning@10.14.1-canary.517.2bd4a12.0 npm install @quenty/templateprovider@11.10.1-canary.517.2bd4a12.0 # or yarn add @quenty/adorneeboundingbox@8.12.1-canary.517.2bd4a12.0 yarn add @quenty/animationprovider@11.10.1-canary.517.2bd4a12.0 yarn add @quenty/chatproviderservice@9.16.1-canary.517.2bd4a12.0 yarn add @quenty/cmdrservice@13.14.1-canary.517.2bd4a12.0 yarn add @quenty/colorpalette@10.13.1-canary.517.2bd4a12.0 yarn add @quenty/gameconfig@12.16.1-canary.517.2bd4a12.0 yarn add @quenty/gameproductservice@14.16.1-canary.517.2bd4a12.0 yarn add @quenty/highlight@10.13.1-canary.517.2bd4a12.0 yarn add @quenty/humanoidspeed@12.14.1-canary.517.2bd4a12.0 yarn add @quenty/inputkeymaputils@14.15.1-canary.517.2bd4a12.0 yarn add @quenty/observablecollection@12.12.1-canary.517.2bd4a12.0 yarn add @quenty/rogue-humanoid@10.14.1-canary.517.2bd4a12.0 yarn add @quenty/rogue-properties@11.14.1-canary.517.2bd4a12.0 yarn add @quenty/scoredactionservice@16.16.1-canary.517.2bd4a12.0 yarn add @quenty/secrets@7.15.1-canary.517.2bd4a12.0 yarn add @quenty/settings@11.15.1-canary.517.2bd4a12.0 yarn add @quenty/settings-inputkeymap@10.17.1-canary.517.2bd4a12.0 yarn add @quenty/soundgroup@1.12.1-canary.517.2bd4a12.0 yarn add @quenty/spawning@10.14.1-canary.517.2bd4a12.0 yarn add @quenty/templateprovider@11.10.1-canary.517.2bd4a12.0 ```
unrooot commented 2 weeks ago

One more thing to note, ObservableSortedList:ObserveIndexByKey() uses Rx.startFrom, which may be worth checking to make sure is also intended.

https://github.com/Quenty/NevermoreEngine/blob/2bd4a120d54eda1d37e3227518fbdf81c469fd45/src/observablecollection/src/Shared/SortedList/ObservableSortedList.lua#L321

unrooot commented 2 weeks ago

it also doesnt seem to respect the observeValue that's passed to it when you add something

Quenty commented 2 weeks ago

See #518 instead

unrooot commented 2 weeks ago

closing since fixed in #518