solarus-games / solarus

This repository was moved to GitLab: https://gitlab.com/solarus-games/solarus
http://www.solarus-games.org
Other
710 stars 134 forks source link

Bug with sprite collision test: callback parameters in wrong order? #1162

Closed Diarandor closed 6 years ago

Diarandor commented 6 years ago

I think that the callback parameters in sprite collision tests for custom entities are wrong. The API says that the third and fourth parameters are, respectively, the sprites of the custom entity and the other entity. However, the order of this two sprites is given in wrong order.

Lua API:

callback (function): A function that will be called when the collision test detects a collision with another entity. This allows you to decide dynamically. This function takes your custom entity and then the other entity as parameters. If the collision test was "sprite", both involved sprites are also passed as third and fourth parameters: the third parameter is the sprite of your custom entity, and the fourth parameter is the sprite of the other entity. This may be useful when your entities have several sprites, otherwise you can just ignore these additional sprite parameters.

Diarandor commented 6 years ago

I have tested this again and I strongly believe that this only happens when the collision is with a sprite of the other_entity that is not the main sprite. When the sprite of the other_entity in the collision is its main sprite, the order is the one mentioned in the Lua API.

Diarandor commented 6 years ago

Check the shield script in the repo of CoS, lines 283-284, where this bug is mentioned: https://github.com/solarus-games/children-of-solarus/blob/dev/data/items/shield.lua There I used a workaround, but if you comment those lines, the bug will reappear.

Diarandor commented 6 years ago

To reproduce the error with the custom shield of CoS, follow these steps (and see https://github.com/solarus-games/children-of-solarus/blob/dev/data/items/shield.lua):

0) The collision test of the shield is defined on the shield_collision_mask custom entity. You may need to read part of the code of the script to understand what I explain below.

1) First, notice that in line 280 I have interchanged the names of the sprites (entity_sprite and shield_sprite) in the wrong position of the function because otherwise the code of my custom shield would not work. The variable entity_sprite has should have the sprite of the shield_collision_mask (because the collision test is defined there). However, the variable entity_sprite is getting both values with collisions: the collision mask sprite and enemy sprites too, which should not happen (there must be a bug). To check that both cases are happening, follow next step and use the shield on an enemy (I don't remember if this happens only for enemies with several sprites or for all enemies).

2) Replace line 284 with this code to print that info and check what I say above:

    if entity_sprite:get_animation_set() ~= "hero/shield_collision_mask" then
      print(entity_sprite:get_animation_set())
    end

or with this code to see the other info:

    if entity_sprite:get_animation_set() == "hero/shield_collision_mask" then
      print(entity_sprite:get_animation_set())
    end
Diarandor commented 6 years ago

Ok, this is an easier way to test this bug. Steps to reproduce it (use the repo of CoS): -Create an enemy and a custom entity in the map with the codes below and put them overlapping themselves. You will see both print messages at the same time, which should not happen. -Script for enemy:

function enemy:on_created()
  local sprite1 = self:create_sprite("enemies/diarandor/goblin_green")
  sprite1:set_animation("walking")
end

-Script for custom entity:

function entity:on_created()
  local sprite = self:create_sprite("enemies/diarandor/chameleon")
  sprite:set_animation("walking")
  self:add_collision_test("sprite", function(this, other, this_sprite, other_sprite)
    if this_sprite:get_animation_set() == "enemies/diarandor/chameleon" then
      print("YEAH!")
    else
      print("NOPE!")
    end
    print(this_sprite:get_animation_set())
  end)
end