RuggeroVisintin / SparkEngineWeb

SparkEngineWeb is a Typescript GameEngine that aims to run in the Browser and in Browser based solutions
https://ruggerovisintin.github.io/SparkEngineWeb/
GNU General Public License v2.0
5 stars 2 forks source link

[BUG] Unique Names issue After deserializing a Scene #376

Closed RuggeroVisintin closed 1 day ago

RuggeroVisintin commented 4 weeks ago

Description When deserializing an Existing Scene, newly created entities reuse the same unique name as some objects created in the scene

Code Snippet

const context = document.getElementById('canvas').getContext
const engine = new SparkEngine.GameEngine({
    context,
    framerate: 30,
    resolution: { width: 1920, height: 1080 }
});
const scene = engine.createScene();
window.onclick = () => {
    window.showOpenFilePicker({
        multiple: false,
        types: [{
            accept: {
                'application/json': ['.spark.json']
            }
        }]
    }).then(([openFileHandle]) => {
        openFileHandle.getFile()
            .then(sceneJson => {
                sceneJson.text().then(data => {
                    console.log('DATA', data)
                    scene.loadFromJson(JSON.parse(data))
                });
            });
    })
}
addEventListener("keypress", (event) => {
    if (event.key === 'a') {
        const gameObject = new SparkEngine.GameObject();

        scene.registerEntity(gameObject);
    }
});
engine.run();

Additional context Screenshot from the current version

Screenshot 2024-08-13 at 18 58 12

Serialized scene being used test.spark.json

RuggeroVisintin commented 4 weeks ago

Request For Change

Context

The reason why entities' names need to be unique in the first place is to allow the identification of an entity through a human-readable unique string.

Initially, names needed to be globally unique, but that didn't make sense anymore once scene deserialization was introduced, as names are serialized in the scene since they can be used to create relationships between entities, the same name might be used across different scenes, causing the check on the global scope to fail.

To overcome this issue, the concept of uniqueness scope was introduced with #331, which led to this bug. This commit tried to address the issue but proved unsuccessful.

Using the data provided in the example and the code, after loading the scene uniqueCounterMap looks as follow

{
  "global": {
    "GameObject1": 0,
    "GameObject2": 0,
    "TriggerEntity": 0
  },
  "2a5e1096-c1b9-42c2-a0dc-eca13b2ca020": {
    "GameObject1": 0,
    "GameObject2": 0,
    "TriggerEntity": 0
  }
}

We can easily observe the entities have been registered in the global scope too, but with the wrong key. When no scene loading is involved, the JSON structure usually looks like this

{
  "global": {
    "GameObject": 1,
    "GameObject1": 0
  },
  "2a5e1096-c1b9-42c2-a0dc-eca13b2ca020": {
    "GameObject": 0,
    "GameObject1": 0,
    "TriggerEntity": 0
  }
}

In general, we would expect the JSON structure to look more like this instead

{
  "global": {
    "GameObject": 2,
    "GameObject1": 1
  },
  "2a5e1096-c1b9-42c2-a0dc-eca13b2ca020": {
    "GameObject": 2,
    "GameObject1": 1,
    "TriggerEntity": 1
  }
}

Issues

Further reflections

Provided names need to be unique only within a given scope, should the default name being provided by the scene, being an entity's parent scope, or should they be self assigned by an entity from the global scope?

Is there any added value other than pure cosmetic to add globally unique names at the entity's level?

Proposals

Unique Names are assigned when an entity is registered in a scene

The first proposal is to introduce a **breaking change** and move the incrementally unique name generation from an entity constructor to the `scene.registerEntity` method, assigning a unique name only when an entity is added to a specific scene. **Considerations** * What if an entity's name is already used in a scene? Should it throw or should it make it unique?

Prepopulate global scope with all entities' types to fix pattern recognition

[This commit](https://github.com/RuggeroVisintin/SparkEngineWeb/commit/ad055afebe40c465448051612353d232b52c5aa4) isn't working due to the lack of proper data at the global scope level. The reason why data isn't there, it's because it isn't populated until required. A way to address the issue could be to let the entity constructor register the type of the entity during construction **Considerations** * Should the entity type be used as unique name only once or every time? Draft PR: https://github.com/RuggeroVisintin/SparkEngineWeb/pull/377
RuggeroVisintin commented 4 weeks ago

Finalizing #377 to solve issue and opening new task to evaluate "Unique Names are assigned when an entity is registered in a scene" proposal