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
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 43 additions & 12 deletions lib/plugins/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -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?

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?

const index = originalVehicle.passengers.indexOf(passenger)
originalVehicle.passengers = originalVehicle.passengers.splice(index, 1)
if (index !== -1) {
originalVehicle.passengers.splice(index, 1)
}
}
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?

vehicle.passengers.push(passenger)
}

if (packet.entityId === bot.entity.id) {
const vehicle = bot.vehicle
Expand All @@ -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

vehicle.passengers = []
}

for (const passengerEntity of passengerEntities) {
if (!passengerEntity) continue

if (!passengerEntity.passengers) {
passengerEntity.passengers = []
}

const originalVehicle = passengerEntity.vehicle
if (originalVehicle !== null) {
if (originalVehicle && originalVehicle.passengers && Array.isArray(originalVehicle.passengers)) {
const index = originalVehicle.passengers.indexOf(passengerEntity)
originalVehicle.passengers = originalVehicle.passengers.splice(index, 1)
if (index !== -1) {
originalVehicle.passengers.splice(index, 1)
}
}

passengerEntity.vehicle = vehicle
if (vehicle !== null) {
if (vehicle && vehicle.passengers && Array.isArray(vehicle.passengers)) {
vehicle.passengers.push(passengerEntity)
}
}
Expand All @@ -856,19 +877,23 @@ function inject (bot) {

// dismounting when the vehicle is gone
bot._client.on('entityGone', (entity) => {
if (!entity) return

if (bot.vehicle === entity) {
bot.vehicle = null
bot.emit('dismount', (entity))
bot.emit('dismount', entity)
}
if (entity.passengers) {
if (entity.passengers && Array.isArray(entity.passengers)) {
for (const passenger of entity.passengers) {
passenger.vehicle = null
if (passenger) {
passenger.vehicle = null
}
}
}
if (entity.vehicle) {
if (entity.vehicle && entity.vehicle.passengers && Array.isArray(entity.vehicle.passengers)) {
const index = entity.vehicle.passengers.indexOf(entity)
if (index !== -1) {
entity.vehicle.passengers = entity.vehicle.passengers.splice(index, 1)
entity.vehicle.passengers.splice(index, 1)
}
}
})
Expand Down Expand Up @@ -975,7 +1000,13 @@ function inject (bot) {
}

function fetchEntity (id) {
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?

bot.entities[id] = entity
return entity
}
return bot.entities[id]
}
}

Expand Down
10 changes: 6 additions & 4 deletions lib/plugins/game.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,12 @@ function inject (bot, options) {
if (packet.worldType && !bot.game.dimension) {
bot.game.dimension = packet.worldType.replace('minecraft:', '')
}
const dimData = bot.registry.dimensionsByName[bot.game.dimension]
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 ?

const dimData = bot.registry.dimensionsByName[bot.game.dimension]
if (dimData) {
bot.game.minY = dimData.minY
bot.game.height = dimData.height
}
}
} else if (bot.supportFeature('dimensionDataIsAvailable')) { // 1.16.2+
const dimensionData = nbt.simplify(packet.dimension)
Expand Down
Loading