Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer terminating a query early #1237

Open
hordurj opened this issue Aug 2, 2024 · 3 comments
Open

Offer terminating a query early #1237

hordurj opened this issue Aug 2, 2024 · 3 comments
Labels
feedback needs-triage Issues that need triaging

Comments

@hordurj
Copy link
Contributor

hordurj commented Aug 2, 2024

The ECS is queried using mach.Entities.Mod.query, which returns a QueryResult that needs to be iterated through to the end.

If that is not done it will cause errors on subsequent queries.

It would be useful to provide a cleanup function, e.g. finish(), done() that could be deferred so that even if iteration is terminates early because of an error or a user decision it would be cleaned up properly.

@hordurj hordurj added feedback needs-triage Issues that need triaging labels Aug 2, 2024
@hordurj
Copy link
Contributor Author

hordurj commented Aug 2, 2024

Below is where it crashed in one application.

mach\src\module\entities.zig:725:38: 0x68c30d in next (collision.exe.obj)
if (state.q.match(archetype)) return archetype;
^
mach\src\module\entities.zig:810:54: 0x65c840 in next (collision.exe.obj)
const archetype = q2.dynamic.next() orelse return null;

I have not been able to create a small example to reproduce.

@hordurj
Copy link
Contributor Author

hordurj commented Aug 3, 2024

A side not regarding the query system and archtypes. When adding components to entities all the intermediate archtypes will be created, this means the query command will return multiple empty archtypes. E.g.

        const player1 = try entities.new();
        try game.set(player1, .name, "jane");
        try game.set(player1, .location, .{ .x = 10.0, .y = 13.1, .z = 21.0 });
        try game.set(player1, .rotation, .{ .degrees = 90 });
        try game.set(player1, .health, 100.0 );
        try shape.set(player1, .rotation, .{ .degrees = 90 });

Now if we query for Game.name then query will return 4 archtypes collections. There are several ways to address that, e.g. prune empty archtypes, set multiple components at a time, i.e. find/or create the archtype that can store all the components.

@hordurj
Copy link
Contributor Author

hordurj commented Aug 3, 2024

The following can be used to clean up using current query interface:

var q = try entities.query(.{ .ids = mach.Entities.Mod.read(.id) });
defer { 
  if (q.dynamic.entities.active_queries.items.len > 0 and !q.dynamic.entities.active_queries.items[q.dynamic.index].finished) {
    while (q.next()) |_| {}
  }
}

The above depends on lot of the internals to check if the query is finished or not, so better to encapsulate that inside the query result struct. E.g. provide a q.finish().

As Zig provides multiple ways to exit a scope, e.g. errros, break, return, there should be some cleanup facility that can be deferred when the query result is created on the stack.

Because next() panics if called after the iteration is completed, it would be useful to have a isFinished(). Alternatively, would be to have next() just return null when the iterator has reached the end. This would match how zig stdlib implements iterators.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feedback needs-triage Issues that need triaging
Projects
None yet
Development

No branches or pull requests

1 participant