Opened 10 years ago
Closed 10 years ago
#6663 closed defect (fixed)
NEVERHOOD: Cannon button sometimes doesn't work (uninitialized variables)
| Reported by: | eriktorbjorn | Owned by: | johndoe123 |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Neverhood |
| Version: | Keywords: | ||
| Cc: | Game: | The Neverhood |
Description
The cannon controls sometimes doesn't work: Nothing happens when I click on the big red button.
This appears to be because Scene3009 adds three collision sprites: SsScene3009FireCannonButton, AsScene3009VerticalIndicator and AsScene3009HorizontalIndicator. But the collision bounds for the second and third are never initialized. Sometimes, by pure chance, they happen to cover the entire screen, and apparently they have higher priority than the fire button.
I don't know why they are collision sprites. Actually, I can't even see them visually in the scene, at least not when the cannon is used early in the game. Perhaps they're only visible later? Either way, it's not obvious to me what the correct fix for this is.
For reference, here is the Valgrind log:
~~~~~ ==10730== Conditional jump or move depends on uninitialised value(s) ==10730== at 0x8B04FD2: Neverhood::Sprite::isPointInside(short, short) (sprite.cpp:70) ==10730== by 0x8B01263: Neverhood::Scene::queryPositionSprite(short, short) (scene.cpp:342) ==10730== by 0x8B00ED9: Neverhood::Scene::update() (scene.cpp:267) ==10730== by 0x8AF3187: Neverhood::Scene3009::update() (module3000.cpp:486) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8AF21FF: Neverhood::Module3000::updateScene() (module3000.cpp:177) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8A81C5D: Neverhood::GameModule::updateModule() (gamemodule.cpp:620) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A6D3F5: Neverhood::NeverhoodEngine::mainLoop() (neverhood.cpp:196) ==10730== ==10730== Conditional jump or move depends on uninitialised value(s) ==10730== at 0x8B04FE2: Neverhood::Sprite::isPointInside(short, short) (sprite.cpp:70) ==10730== by 0x8B01263: Neverhood::Scene::queryPositionSprite(short, short) (scene.cpp:342) ==10730== by 0x8B00ED9: Neverhood::Scene::update() (scene.cpp:267) ==10730== by 0x8AF3187: Neverhood::Scene3009::update() (module3000.cpp:486) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8AF21FF: Neverhood::Module3000::updateScene() (module3000.cpp:177) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8A81C5D: Neverhood::GameModule::updateModule() (gamemodule.cpp:620) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A6D3F5: Neverhood::NeverhoodEngine::mainLoop() (neverhood.cpp:196) ==10730== ==10730== Conditional jump or move depends on uninitialised value(s) ==10730== at 0x8B04FF2: Neverhood::Sprite::isPointInside(short, short) (sprite.cpp:70) ==10730== by 0x8B01263: Neverhood::Scene::queryPositionSprite(short, short) (scene.cpp:342) ==10730== by 0x8B00ED9: Neverhood::Scene::update() (scene.cpp:267) ==10730== by 0x8AF3187: Neverhood::Scene3009::update() (module3000.cpp:486) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8AF21FF: Neverhood::Module3000::updateScene() (module3000.cpp:177) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8A81C5D: Neverhood::GameModule::updateModule() (gamemodule.cpp:620) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A6D3F5: Neverhood::NeverhoodEngine::mainLoop() (neverhood.cpp:196) ==10730== ==10730== Conditional jump or move depends on uninitialised value(s) ==10730== at 0x8B05002: Neverhood::Sprite::isPointInside(short, short) (sprite.cpp:70) ==10730== by 0x8B01263: Neverhood::Scene::queryPositionSprite(short, short) (scene.cpp:342) ==10730== by 0x8B00ED9: Neverhood::Scene::update() (scene.cpp:267) ==10730== by 0x8AF3187: Neverhood::Scene3009::update() (module3000.cpp:486) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8AF21FF: Neverhood::Module3000::updateScene() (module3000.cpp:177) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A89A74: Neverhood::Module::updateChild() (module.cpp:104) ==10730== by 0x8A81C5D: Neverhood::GameModule::updateModule() (gamemodule.cpp:620) ==10730== by 0x8A7F952: Neverhood::Entity::handleUpdate() (entity.cpp:64) ==10730== by 0x8A6D3F5: Neverhood::NeverhoodEngine::mainLoop() (neverhood.cpp:196) ==10730== ~~~~~
Ticket imported from: bugs/6663.
Attachments (1)
Change History (5)
by , 10 years ago
| Attachment: | neverhood-win.008 added |
|---|
comment:1 by , 10 years ago
comment:2 by , 10 years ago
| Owner: | set to |
|---|
comment:3 by , 10 years ago
Some fields in Sprite weren't correctly inizialized, they're all set to 0 now.
comment:4 by , 10 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Replicated here with Linux x86_64.
I think the root cause of the issue is that the Sprite class defines a member variable for the Collision Bounds, but sets no default valueas NRect has no default constructor to set it to zero... as opposed to NDrawRect which does: https://github.com/scummvm/scummvm/blob/master/engines/neverhood/sprite.h#L94
See here and here for NRect vs. NDrawRect: https://github.com/scummvm/scummvm/blob/master/engines/neverhood/graphics.h#L43 https://github.com/scummvm/scummvm/blob/master/engines/neverhood/graphics.h#L68
Beyond this, the values that should be set for the collision bounds for SsScene3009FireCannonButton, AsScene3009VerticalIndicator and AsScene3009HorizontalIndicator is a different issue... but this will at least ensure that they and other StaticSprite / Sprite usage does not end up with uninitialized and unstable values.