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

Fix potential null vehicle passengers array in entities plugin #3575

Closed
wants to merge 5 commits into from

Conversation

TheRealPerson98
Copy link

@TheRealPerson98 TheRealPerson98 commented Jan 30, 2025

Fix crash caused by undefined vehicle entity in entities.js

This commit fixes a crash occurring when attempting to access vehicle.passengers
on an undefined vehicle entity. The issue was triggered in:

lib\plugins\entities.js:841

TypeError: Cannot read properties of undefined (reading 'passengers')
    at Client.<anonymous> (C:\ops/mineflayer\lib\plugins\entities.js:841:17)
    at Client.emit (node:events:517:28)
    at emitPacket (C:\opsminecraft-protocol\src\client.js:84:12)
    at Array.forEach (<anonymous>)
    at FullPacketParser.<anonymous> (C:REDACTEDnode_modules\minecraft-protocol\src\client.js:99:26)
    at FullPacketParser.emit (node:events:517:28)
    at addChunk (C:REDACTEDnode_modules\readable-stream\lib\internal\streams\readable.js:323:12)
    at readableAddChunk (C:REDACTEDnode_modules\readable-stream\lib\internal\streams\readable.js:300:9)
    at Readable.push (C:REDACTEDnode_modules\readable-stream\lib\internal\streams\readable.js:246:10)
    at FullPacketParser._transform (C:REDACTEDnode_modules\protodef\src\serializer.js:89:10)

- Remove unnecessary semicolons
- Simplify dismount event emission
- Ensure consistent null and array checks for vehicle and passenger entities
@TheRealPerson98
Copy link
Author

I didn't modify beds, but I can fix the error in beds if needed. Let me know!

@rom1504
Copy link
Member

rom1504 commented Jan 30, 2025

The more error you fix the better

@TheRealPerson98
Copy link
Author

OK give me 30 mins

@TheRealPerson98
Copy link
Author

Wait that test passed local

- Remove commented initialization comment
- Simplify comments for passenger entity checks
- Maintain existing logic for removing passengers from vehicles
@TheRealPerson98
Copy link
Author

Ok cool their

@@ -806,13 +806,21 @@ function inject (bot) {
const passenger = fetchEntity(packet.entityId)
const vehicle = packet.vehicleId === -1 ? null : fetchEntity(packet.vehicleId)

// Initialize passengers arrays if they don't exist
if (!passenger.passengers) passenger.passengers = []
Copy link
Member

Choose a reason for hiding this comment

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

can this happen? if so why? can we initialize this when we create the entity?

@@ -806,13 +806,21 @@ function inject (bot) {
const passenger = fetchEntity(packet.entityId)
const vehicle = packet.vehicleId === -1 ? null : fetchEntity(packet.vehicleId)

// Initialize passengers arrays if they don't exist
if (!passenger.passengers) passenger.passengers = []
if (vehicle && !vehicle.passengers) vehicle.passengers = []
Copy link
Member

Choose a reason for hiding this comment

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

same question

const originalVehicle = passenger.vehicle
if (originalVehicle !== null) {
if (originalVehicle && originalVehicle.passengers && Array.isArray(originalVehicle.passengers)) {
Copy link
Member

Choose a reason for hiding this comment

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

can this happen? is it a minecraft bug?

}
passenger.vehicle = vehicle
vehicle.passengers.push(passenger)
if (vehicle && vehicle.passengers && Array.isArray(vehicle.passengers)) {
Copy link
Member

Choose a reason for hiding this comment

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

can vehicle.passengers not be an array?

@@ -830,14 +838,27 @@ function inject (bot) {
const passengerEntities = passengers.map((passengerId) => fetchEntity(passengerId))
const vehicle = entityId === -1 ? null : bot.entities[entityId]

if (vehicle && !vehicle.passengers) {
Copy link
Member

Choose a reason for hiding this comment

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

same q as above

return bot.entities[id] || (bot.entities[id] = new Entity(id))
if (!bot.entities[id]) {
const entity = new Entity(id)
entity.passengers = []
Copy link
Member

Choose a reason for hiding this comment

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

why not put this directly in prismarine entity ?

if we do that here why do we need to check passengers is an array above?

if (dimData) {
bot.game.minY = dimData.minY
bot.game.height = dimData.height
if (bot.registry.dimensionsByName) {
Copy link
Member

Choose a reason for hiding this comment

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

can this be empty? is it a bug in minecraft ?

@TheRealPerson98
Copy link
Author

sorry way to much effort GL tho love the project

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