Mineclonia
/
Mineclonia
Archived
12
17
Fork 13
Code Issues 105 Pull Requests 19 Projects Releases Wiki Activity

ENTITIES/mcl_boats: Fix mob-in-boat crash in Minetest 5.5-dev #249

Merged
cora merged 1 commits from fix-boat-crash-in-minetest-5.5-dev into master 2022-01-27 01:50:10 +01:00
Conversation 0 Commits 1 Files Changed 1 +2 -2
erlehmann commented 2022-01-26 21:52:45 +01:00
Owner
Copy Link
Problem

TRACKING ISSUE: #241

In Minetest 5.4.1, calling get_player_control() on a mob returned the
empty string. Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d
changed this, so now calling get_player_control() on a mob returns nil.

As mcl_boats defines boats that can have a player or a mob as a driver,
code like the following crashes with a changed get_player_control() API:

local ctrl = driver:get_player_control()
if ctrl.sneak then
    detach_object(driver, true)
end

Furthermore, once a world has crashed, joining it near a mob that is the
driver of a boat with such control code immediately crashes again.

Solution

When I reported this bug to Minetest, several Minetest core developers
stated that they disliked the old API and proposed other return values
for calling a mob's get_player_control() function – all different from
the empty string. Since I have some doubts that this bug will be fixed
in Minetest 5.5.0, boat code must take into account a nil return value.

Details

First time in my life that I proposed a fix to a crash and got only downvotes:
https://github.com/minetest/minetest/pull/11990

Testing Steps
Verify Bug
  1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac.
    2, Find or spawn an mob, e.g. a sheep.
  2. Place a boat near the mob so that the mob is in the boat.
  3. Exit Minetest.
  4. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit b1b96e3fac.
  5. Verify that the game crashes.
Verify Fix
  1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac.
    2, Find or spawn an mob, e.g. a sheep.
  2. Place a boat near the mob so that the mob is in the boat.
  3. Exit Minetest.
  4. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit f89c52dff6.
  5. Verify that the game does not crash.
  6. Verify that you can still do the following things with a boat without a mob in it:
    • Enter it (right click)
    • Exit it (sneak key)
    • Move forward (up key)
    • Move backward (down key)
    • Turn left (left key)
    • Turn right (right key)
    • Coast along when the boat has speed
  7. Verify that you can still do the following things with a boat with a mob in it:
    • Enter it (right click)
    • Exit it (sneak key)
    • Move forward (up key)
    • Move backward (down key)
    • Turn left (left key)
    • Turn right (right key)
    • Coast along when the boat has speed
##### Problem TRACKING ISSUE: https://git.minetest.land/Mineclonia/Mineclonia/issues/241 In Minetest 5.4.1, calling get_player_control() on a mob returned the empty string. Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d changed this, so now calling get_player_control() on a mob returns nil. As mcl_boats defines boats that can have a player or a mob as a driver, code like the following crashes with a changed get_player_control() API: ``` local ctrl = driver:get_player_control() if ctrl.sneak then detach_object(driver, true) end ``` Furthermore, once a world has crashed, joining it near a mob that is the driver of a boat with such control code immediately crashes again. ##### Solution When I reported this bug to Minetest, several Minetest core developers stated that they disliked the old API and proposed other return values for calling a mob's get_player_control() function – all different from the empty string. Since I have some doubts that this bug will be fixed in Minetest 5.5.0, boat code must take into account a nil return value. ##### Details First time in my life that I proposed a fix to a crash and got only downvotes: https://github.com/minetest/minetest/pull/11990 ##### Testing Steps ###### Verify Bug 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. 2, Find or spawn an mob, e.g. a sheep. 3. Place a boat near the mob so that the mob is in the boat. 4. Exit Minetest. 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. 6. Verify that the game crashes. ###### Verify Fix 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. 2, Find or spawn an mob, e.g. a sheep. 3. Place a boat near the mob so that the mob is in the boat. 4. Exit Minetest. 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit f89c52dff6e12abd17b295199502bca665f71d50. 6. Verify that the game does not crash. 7. Verify that you can still do the following things with a boat without a mob in it: * Enter it (right click) * Exit it (sneak key) * Move forward (up key) * Move backward (down key) * Turn left (left key) * Turn right (right key) * Coast along when the boat has speed 8. Verify that you can still do the following things with a boat with a mob in it: * Enter it (right click) * Exit it (sneak key) * Move forward (up key) * Move backward (down key) * Turn left (left key) * Turn right (right key) * Coast along when the boat has speed
erlehmann added 1 commit 2022-01-26 21:52:48 +01:00
f89c52dff6
Fix mob-in-boat crash in Minetest 5.5-dev 
In Minetest 5.4.1, calling get_player_control() on a mob returned the
empty string. Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d
changed this, so now calling get_player_control() on a mob returns nil.

As mcl_boats defines boats that can have a player or a mob as a driver,
code like the following crashes with a changed get_player_control() API:

local ctrl = driver:get_player_control()
if ctrl.sneak then
    detach_object(driver, true)
end

Furthermore, once a world has crashed, joining it near a mob that is the
driver of a boat with such control code immediately crashes again.

When I reported this bug to Minetest, several Minetest core developers
stated that they disliked the old API and proposed other return values
for calling a mob's get_player_control() function – all different from
the empty string. Since I have some doubts that this bug will be fixed
in Minetest 5.5.0, boat code must take into account a nil return value.

Minetest issue: https://github.com/minetest/minetest/issues/11989
erlehmann requested review from cora 2022-01-26 21:55:42 +01:00
erlehmann requested review from e 2022-01-26 21:55:42 +01:00
cora approved these changes 2022-01-27 01:49:57 +01:00
cora left a comment
Member
Testing Steps
Verify Bug
  • 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac.
  • 2, Find or spawn an mob, e.g. a sheep.
  • 3. Place a boat near the mob so that the mob is in the boat.
  • 4. Exit Minetest.
  • 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit b1b96e3fac.
  • 6. Verify that the game crashes.
Verify Fix
  • 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac.
  • 2, Find or spawn an mob, e.g. a sheep.
  • 3. Place a boat near the mob so that the mob is in the boat.
  • 4. Exit Minetest.
  • 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit f89c52dff6.
  • 6. Verify that the game does not crash.
  • 7. Verify that you can still do the following things with a boat without a mob in it:
  • * Enter it (right click)
  • * Exit it (sneak key)
  • * Move forward (up key)
  • * Move backward (down key)
  • * Turn left (left key)
  • * Turn right (right key)
  • * Coast along when the boat has speed
  • 8. Verify that you can still do the following things with a boat with a mob in it:
  • * Enter it (right click)
  • * Exit it (sneak key)
  • * Move forward (up key)
  • * Move backward (down key)
  • * Turn left (left key)
  • * Turn right (right key)
  • * Coast along when the boat has speed
##### Testing Steps ###### Verify Bug - [x] 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. - [x] 2, Find or spawn an mob, e.g. a sheep. - [x] 3. Place a boat near the mob so that the mob is in the boat. - [x] 4. Exit Minetest. - [x] 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. - [x] 6. Verify that the game crashes. ###### Verify Fix - [x] 1. Enter a world in Minetest 5.4.1 with Mineclonia commit b1b96e3fac6e1fc93356d86132e6b7a65419fd47. - [x] 2, Find or spawn an mob, e.g. a sheep. - [x] 3. Place a boat near the mob so that the mob is in the boat. - [x] 4. Exit Minetest. - [x] 5. Enter the previous world in Minetest commit 5eb45e1ea03c6104f007efec6dd9c351f310193d with Mineclonia commit f89c52dff6e12abd17b295199502bca665f71d50. - [x] 6. Verify that the game does not crash. - [x] 7. Verify that you can still do the following things with a boat without a mob in it: - [x] * Enter it (right click) - [x] * Exit it (sneak key) - [x] * Move forward (up key) - [x] * Move backward (down key) - [x] * Turn left (left key) - [x] * Turn right (right key) - [x] * Coast along when the boat has speed - [x] 8. Verify that you can still do the following things with a boat with a mob in it: - [x] * Enter it (right click) - [x] * Exit it (sneak key) - [x] * Move forward (up key) - [x] * Move backward (down key) - [x] * Turn left (left key) - [x] * Turn right (right key) - [x] * Coast along when the boat has speed
cora merged commit 6abdbbbd13 into master 2022-01-27 01:50:10 +01:00
cora deleted branch fix-boat-crash-in-minetest-5.5-dev 2022-01-27 01:50:12 +01:00
erlehmann removed review request for e 2022-01-27 01:57:41 +01:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
cora
Labels
Clear labels   
blocker

blocks release until resolved

  
bug

   
code quality

   
confirmed

   
critical

crashes, catastrophic bugs

  
discussion

   
high priority

   
incompatibility

incompatible with mcl2

  
incomplete feature

   
invalid

   
low priority

   
missing feauture

   
needs testing

   
packet spam

   
performance

   
project

meta-issues

  
regression

   
translations

   
unconfirmed
  
in review

   
ready for review
No Label
blocker
bug
code quality
confirmed
critical
discussion
high priority
incompatibility
incomplete feature
invalid
low priority
missing feauture
needs testing
packet spam
performance
project
regression
translations
unconfirmed
in review
ready for review
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: Mineclonia/Mineclonia#249
No description provided.
Delete Branch "fix-boat-crash-in-minetest-5.5-dev"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?