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

[Signal] All signals should pass the EntityManagerT that was used #67

Open
iK4tsu opened this issue Sep 6, 2021 · 0 comments
Open

[Signal] All signals should pass the EntityManagerT that was used #67

iK4tsu opened this issue Sep 6, 2021 · 0 comments
Labels
breaking change No backwards compatibility refactor Improvements or small fixes of the source code

Comments

@iK4tsu
Copy link
Contributor

iK4tsu commented Sep 6, 2021

Although most of the time the user will, most likely, use only 1 EntityManagerT, the library supports multiple to be defined. If the user defines more than one he may also want to use the same callback for different EntityManagerT instances.

There are some problems with the current implementation:

  • if the callback is a function pointer, the user has no way to access EntityManagerT
  • if the callback is used across multiple instances the user has no way to know which was used

The implementation should change to the following signature:

void delegate(scope EntityManagerT, Entity)

The EntityManagerT should be scope to prevent it from escaping. This currently has some issues with DIP1000 and @safe code. When passed as scope all accesses must be scope compliant, requiring all functions to be defined with scope. This is an issue because it does not make sense to define the methods as scope for general use. Templating the functions might works in cases, from what I've tested, but I don't know if it can work as a solution for the entire lib.

An alternative to this is to refactor EntityManagerT to be a struct and not a class. With the struct approach callbacks could be defined as:

void delegate(ref EntityManagerT, Entity)

This fixes almost all issues with not much refactoring needing to be made. The downside is that the user will have to work with raw pointers.

@iK4tsu iK4tsu added refactor Improvements or small fixes of the source code breaking change No backwards compatibility labels Sep 6, 2021
@iK4tsu iK4tsu changed the title [Signal] All signals should pass the EntityManagerT was used [Signal] All signals should pass the EntityManagerT that was used Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change No backwards compatibility refactor Improvements or small fixes of the source code
Projects
None yet
Development

No branches or pull requests

1 participant