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

EventChange implementation mistakes #22

Closed
Gerard097 opened this issue Nov 22, 2018 · 1 comment
Closed

EventChange implementation mistakes #22

Gerard097 opened this issue Nov 22, 2018 · 1 comment

Comments

@Gerard097
Copy link

EventChange isn't properly implemented. As a template class, it should have its methods defined in the same file that was declared, or including the .inl (cpp) at the end of the file (hpp).

Also the EventTriggered has a typo i guess.

T newValue = &m_reference; //&

I think that the real intention was to obtain the value of the reference

T newValue = *m_reference; //*

and also the if (newValue == nullptr) has no sense in this context.

Notice that this implementation will only notify one change if any per frame. I'd recommend also adding another method to achieve this kind of behavior but for every change like an Observer-Subject pattern or wrapping the change value inside a class as suggested in this SO https://stackoverflow.com/a/3159198/7908019.

@mattparks
Copy link
Member

Thanks for finding this issue, should now be fixed with 777db1c. A assignment hook is probably not needed for this event type as it is used to check if the value is different from the last update time, not if it has ever had a change between the updates.

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

No branches or pull requests

2 participants