search

yohamta / donburi

Just another ECS library for Go/Ebitengine
https://pkg.go.dev/github.com/yohamta/donburi
Other
232 stars 21 forks source link

Query callback is called twice for the same entity #119

Open mrombout opened 5 months ago

mrombout commented 5 months ago

It appears that sometimes the callback for a query is called twice when dynamically adding and removing components from an entity. Is it perhaps a bug, or caused by misuse of the API?

Self-contained example

The self-contained example below hopefully demonstrates the problem. I've also put the example on Go Playground.

package main

import (
    "log"

    "github.com/yohamta/donburi"
    "github.com/yohamta/donburi/filter"
)

type PositionData struct {
    X int
    Y int
}

var Position = donburi.NewComponentType[PositionData]()

var Player = donburi.NewTag()

type DoActionData struct {
    action string
}

var DoAction = donburi.NewComponentType[DoActionData]()

func main() {
    world := donburi.NewWorld()

    playerEntity := world.Create(Player, Position)

    log.Println("=== ROUND 1")
    receivePlayerInput(world)
    processActions(world, playerEntity)
    log.Println("=== ROUND 2")
    receivePlayerInput(world)
    processActions(world, playerEntity)
}

func receivePlayerInput(world donburi.World) {
    log.Println("receivePlayerInput")

    query := donburi.NewQuery(filter.Contains(Player, Position))
    log.Printf("receivePlayerInput query count: %d\n", query.Count(world))
    query.Each(world, func(e *donburi.Entry) {
        log.Printf("receivePlayerInput query callback for %s\n", e)
        log.Printf("adding DoAction to %s\n", e)
        donburi.Add(e, DoAction, &DoActionData{
            action: "do thing",
        })
    })
}

func processActions(world donburi.World, playerEntity donburi.Entity) {
    playerEntry := world.Entry(playerEntity)
    log.Printf("removing DoAction from %s\n", playerEntry)
    donburi.Remove[DoActionData](playerEntry, DoAction)
}

Output

2024/01/23 00:03:30 === ROUND 1
2024/01/23 00:03:30 receivePlayerInput
2024/01/23 00:03:30 receivePlayerInput query count: 1
2024/01/23 00:03:30 receivePlayerInput query callback for Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData}, Valid: true}
2024/01/23 00:03:30 adding DoAction to Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData}, Valid: true}
2024/01/23 00:03:30 removing DoAction from Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData, DoActionData}, Valid: true}
2024/01/23 00:03:30 === ROUND 2
2024/01/23 00:03:30 receivePlayerInput
2024/01/23 00:03:30 receivePlayerInput query count: 1
2024/01/23 00:03:30 receivePlayerInput query callback for Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData}, Valid: true}
2024/01/23 00:03:30 adding DoAction to Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData}, Valid: true}
2024/01/23 00:03:30 receivePlayerInput query callback for Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData, DoActionData}, Valid: true}
2024/01/23 00:03:30 adding DoAction to Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData, DoActionData}, Valid: true}
2024/01/23 00:03:30 removing DoAction from Entry: {Entity: {id: 1, version: 0}, Layout: {, PositionData, DoActionData}, Valid: true}

Actual behavior

In ROUND 1 the query behaves as I would expect. There is 1 entity that has both the Player and Position components, the callback is called once and the DoAction component is added. It is then processed and the DoAction component is removed from the entity.

In ROUND 2 the entity should be back in the state it was before with only the Player and Position components. The query .Count() returns 1, which is correct. But the query now appears to call the callback twice for the same entity, but with different layouts. Once without the DoAction and once without the DoAction.

Expected behavior

I would expect ROUND 1 and ROUND 2 to behave the same because the component is added and removed in each round.

yohamta commented 5 months ago

Thanks a lot for submitting the issue with such details, that's really helpful. In donburi, for performance reasons, a SwapRemove is performed internally on a slice. Therefore, when Remove is executed, the last element is inserted at the index of the removed element. This might be causing the behavior you're observing 🤔 I think we might have to investigate deeper to identify the cause.

bradbeam commented 5 months ago

I think I'm running into something similar, but for my use case it is only around modifications ( namely transform.SetWorldPosition(card, localPosition) and transform.AppendChild(stack, card, false). So far I'm suspecting it's related to the archetypes and how they get generated when the entity changes. In my case, calling transform.AppendChild calls child.AddComponent(parentComponent) which causes a new archetype to be created.

We discover and iterate through the archetypes via our calls to Each. So in the first run, the only archetype that World knows about is PositionData. After the add of DoActionData, the world knows about two archetypes, PositionData and one for PositionData, DoActionData. So in the second run, the query matches both archetypes and you wind up with seemingly duplicate entries.

Here's a modification of your example to highlight it ~ https://go.dev/play/p/F0vrtZdKLol (note, you can ignore the move of query outside of the receive player input; in playing around I came across the note here https://pkg.go.dev/github.com/yohamta/donburi@v1.3.12/query#Query and was curious if it was related, but don't believe it is )

Thoughts?

some refs: