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

Minecraft 1.20.2 support #3262

Merged
merged 18 commits into from
Jan 14, 2024
Merged

Minecraft 1.20.2 support #3262

merged 18 commits into from
Jan 14, 2024

Conversation

rom1504
Copy link
Member

@rom1504 rom1504 commented Dec 30, 2023

migrated from #3243

@rom1504 rom1504 mentioned this pull request Dec 30, 2023
@@ -212,6 +213,14 @@ function inject (bot) {
entity.yaw = conv.fromNotchianYawByte(packet.yaw)
entity.pitch = conv.fromNotchianPitchByte(packet.pitch)
entity.objectData = packet.objectData

if (entity.type === 'player') {
Copy link
Member Author

Choose a reason for hiding this comment

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

that looks pretty suspicious, not sure if it's right

Copy link
Contributor

Choose a reason for hiding this comment

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

I suspect it is correct based on existing code using the opposite condition. https://github.com/PrismarineJS/mineflayer/blob/master/lib/plugins/entities.js#L42

Not sure if what its being used for is correct tho.

Copy link
Member

Choose a reason for hiding this comment

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

named_entity_spawn was removed, so players spawn with the generic spawn_entity packet. However there are alot of unhandled fields that are not currently accounted for -

bot._client.on('named_entity_spawn', (packet) => {

* Add Chunk Batch support

* Fix linting errors.

* I really hate the linter right now.

* Comment what chunkbatches are for.

* Update chunk batch calculation to use Vanilla's calc but updated for Milliseconds vs Nanoseconds

* Fix usage of variables starting with capital letters
@wgaylord
Copy link
Contributor

wgaylord commented Jan 2, 2024

I think the only thing left is to fix what ever is breaking the External Bed and Elytra tests. Then fix the Internal tests since they are all totally failing.

@wgaylord
Copy link
Contributor

wgaylord commented Jan 2, 2024

Found PrismarineJS/minecraft-data#817 to be why both the External Bed and Elytra tests are failing

@extremeheat
Copy link
Member

extremeheat commented Jan 2, 2024

Also, node v20 is failing now if anyone has time to look into it:

> mocha --reporter spec --exit

/workspaces/mineflayer/test/externalTests/gamemode.js:6
  bot.test.becomeCreative()
           ^

TypeError: Cannot read properties of undefined (reading 'becomeCreative')

Likely related to timing/scheduling

Also, once entity tests are fixed someone should test some examples in-game to see important untested things are working ok (like pathfinder)

@wgaylord
Copy link
Contributor

wgaylord commented Jan 4, 2024

okay, so the major thing stopping this is changes to the entity spawning packet in both Mineflayer and in the internal tests
https://github.com/PrismarineJS/mineflayer/blob/mc-1.20.2/lib/plugins/entities.js#L132-L157 is how we used to handle player spawns. For 1.20.2+ players spawn using the same as other mobs, partially implemented here https://github.com/PrismarineJS/mineflayer/blob/mc-1.20.2/lib/plugins/entities.js#L196-L224 We need to fill out the rest of the fields for the player.

This same sort of change needs to happen on in the integral tests based on version. Where https://github.com/PrismarineJS/mineflayer/blob/mc-1.20.2/test/internalTest.js#L635-L647 and https://github.com/PrismarineJS/mineflayer/blob/mc-1.20.2/test/internalTest.js#L700-L712 need to be setup so if we are not 1.20.2+ they are used and if we are 1.20.2+ something like https://github.com/PrismarineJS/mineflayer/blob/mc-1.20.2/test/internalTest.js#L786-L800 is used (of course with the correct values.)

@rom1504
Copy link
Member Author

rom1504 commented Jan 4, 2024

@wgaylord anything unclear about it or you just need to do it?

@wgaylord
Copy link
Contributor

wgaylord commented Jan 5, 2024

Mostly just need to do it, wanted to document exactly what was still needed, incase I don't get the time to work on it.

@extremeheat
Copy link
Member

Ok, should be passing now. Required changes were:

  • server.on login (packet) -> playerJoin (nmp event) to handle play state
  • named_entity_spawn is now done with spawn_entity packet
  • dimension codec is sent outside login packet now (before play state)

currentItem: -1,
metadata: []
})
if (bot.registry.version['>=']('1.20.2')) {
Copy link
Member Author

Choose a reason for hiding this comment

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

would be good to make that a feature

Copy link
Member

@extremeheat extremeheat Jan 9, 2024

Choose a reason for hiding this comment

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

done, looks like removing dupe PrismarineJS/minecraft-data@587a134 caused range to be incorrect breaking chunk lighting, just fixed that

@rom1504
Copy link
Member Author

rom1504 commented Jan 8, 2024

great

@rom1504
Copy link
Member Author

rom1504 commented Jan 9, 2024

Remaining here, test some examples manually

  • Inventory.js
  • digger
  • chest
  • check previous PRs

@extremeheat extremeheat mentioned this pull request Jan 12, 2024
@rom1504
Copy link
Member Author

rom1504 commented Jan 14, 2024

I tested digger, chest, inventory, jumper, viewer and pathfinder, all working

@rom1504 rom1504 merged commit 2ff9919 into master Jan 14, 2024
19 of 21 checks passed
@rom1504
Copy link
Member Author

rom1504 commented Jan 14, 2024

/makerelease

@rom1504bot rom1504bot mentioned this pull request Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

4 participants