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

zflecs: Added zflecs_init and zflecs_fini. #421

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Srekel
Copy link
Contributor

@Srekel Srekel commented Oct 10, 2023

Hi, this PR is very open to feedback but I think the core of it is reasonable. It moves a few lines out of zflecs.init and zflecs.fini to two new functions, zflecs.zflecs_init and zflecs.zflecs_fini.

The purpose/result is twofold:

  • Take a step towards zflecs supporting more than one world by moving away things that doesn't strictly have to do with world lifetimes from the world lifetime functions.
  • Proper setup of the abort function. Previously, in the order it was done, meant that it would in fact get overridden by flecs to the default handler and was therefore in fact not used at all. Now if flecs asserts while debugging, we will end up at the @breakpoint line.

Here's what my game code looks like:


    ecs.zflecs_init();
    defer ecs.zflecs_fini();
    var ecsu_world = ecsu.World.init();
    defer ecsu_world.deinit();

@hazeycode
Copy link
Member

Hey @Srekel are you still wanting to merge this?

I'm not very familiar with flecs but this refactor looks good to me. Although I wonder if the init and fini should be moved to World.init and World.fini and then have zflecs_init and zflecs_fini take the names init and fini, or am I reading this wrong?

@Srekel
Copy link
Contributor Author

Srekel commented Jan 31, 2024

I'll need to get back to you on this a bit later once I've double-checked the changes :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants