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

Export changedSlots computation to mineflayer #102

Merged
merged 7 commits into from
Aug 5, 2023
Merged

Export changedSlots computation to mineflayer #102

merged 7 commits into from
Aug 5, 2023

Conversation

kaduvert
Copy link
Contributor

@kaduvert kaduvert commented Jul 30, 2023

move the computation for changedSlots to mineflayer and implement returning of changedSlots from clicks directly to save expensive computation

@kaduvert kaduvert changed the title Return changed slots Export changedSlots computation to mineflayer Jul 30, 2023
@rom1504
Copy link
Member

rom1504 commented Aug 5, 2023

can you explain the intent of this change? I don't understand

@kaduvert
Copy link
Contributor Author

kaduvert commented Aug 5, 2023

so returning differences between windows doesn't really belong in prismarine-windows, it does calculation which is of importance only for mineflayer because it needs to know changedSlots for the packets

i included this in #74 to get rid of version issues with mineflayer, but the behaviour is far from optimal

that's what this pr tries to adress

@kaduvert
Copy link
Contributor Author

kaduvert commented Aug 5, 2023

it also clones the inv slots by using JSON.parse(JSON.stringify(slots)) on every click, which is kind of a waste.
so i included direct returning of changedSlots from some clicks to save on this computation in some scenarios

@rom1504
Copy link
Member

rom1504 commented Aug 5, 2023

can you git depend on this in the mineflayer PR so we can see it breaks nothing?

@rom1504
Copy link
Member

rom1504 commented Aug 5, 2023

CI fails

@rom1504
Copy link
Member

rom1504 commented Aug 5, 2023

so returning differences between windows doesn't really belong in prismarine-windows, it does calculation which is of importance only for mineflayer because it needs to know changedSlots for the packets

tbh I don't really understand this. What do you think should belong in mineflayer vs pwindows ?

@kaduvert
Copy link
Contributor Author

kaduvert commented Aug 5, 2023

currently, clicks return changedSlots directly in the format expected for minecraft packets above 1.17.
while i think returning changedSlots from acceptClick is good, returning it in a format which is intended for use in packets is not. that's mineflayer's job.

it currently returns:

[{location: i, item: Item.toNotch(window.slots[i])}, ...]

but i think more appropriate would be:

[i, ...]

and then to let mineflayer handle the serialization for the packets

@rom1504 rom1504 merged commit 98c3d66 into PrismarineJS:master Aug 5, 2023
2 checks passed
@rom1504
Copy link
Member

rom1504 commented Aug 5, 2023

/makerelease 2.8.0

@rom1504bot rom1504bot mentioned this pull request Aug 5, 2023
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.

2 participants