-
-
Notifications
You must be signed in to change notification settings - Fork 943
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
Minecraft 1.20.2 support #3262
Conversation
lib/plugins/entities.js
Outdated
@@ -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') { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 -
mineflayer/lib/plugins/entities.js
Line 132 in a0a5777
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
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. |
Found PrismarineJS/minecraft-data#817 to be why both the External Bed and Elytra tests are failing |
Also, node v20 is failing now if anyone has time to look into it:
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) |
okay, so the major thing stopping this is changes to the entity spawning packet in both Mineflayer and in the internal tests 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.) |
@wgaylord anything unclear about it or you just need to do it? |
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. |
Ok, should be passing now. Required changes were:
|
test/internalTest.js
Outdated
currentItem: -1, | ||
metadata: [] | ||
}) | ||
if (bot.registry.version['>=']('1.20.2')) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
great |
Remaining here, test some examples manually
|
I tested digger, chest, inventory, jumper, viewer and pathfinder, all working |
/makerelease |
migrated from #3243