You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently, the Event Manager is written in two files:
event_manager.hpp contains the macro, and all event definitions;
event_manager.cpp covers the implementation of said events.
This giant monolith becomes problematic for several reasons:
upon changing anything in one of those two files, around a fifth of the project is recompiled (as of today, 147/540 files);
this creates longer wait times, especially when working on implementing events, which is bad for DX;
ultimately, Imhex being based on an event-loop, the evergrowing monolith will only cause more problems.
Also, some events have evolved since their first implementation, and they do not reflect their initial name anymore.
For example, see EventProviderOpened as of the writing of this issue.
How will this feature be useful to you and others?
This is an internal change, which aims to improve the DX and the overall code architecture. No impact on UX is expected.
The feature request aims to streamline the Event Manager in several aspects.
Firstly, split the event definitions into meaningful categories (e.g. events_internal.hpp, events_achievements.hpp, etc.) so that code is more modular and can be easily adapted to fit new needs (like new categories). This would also avoid a clog-up of the current large file that contains all the definitions.
Secondly, use this request as leverage to carefully re-read, analyse, and possibly refactor some events. As stated in the section above, some events don't fit their initial names anymore, or have completely derived from their initial goal.
Lastly, it would be useful to have short but meaningful descriptions of the events in code comments above their respective definitions. Something that new contributors might struggle with is knowing what is expected to be passed in events when posting them, so describing what exactly those parameters serve could help reading and understanding ImHex's architecture. This will lengthen the files, but I think it's for the greater good in the long run.
Request Type
I can provide a PoC for this feature or am willing to work on it myself and submit a PR
What feature would you like to see?
Currently, the Event Manager is written in two files:
event_manager.hpp
contains the macro, and all event definitions;event_manager.cpp
covers the implementation of said events.This giant monolith becomes problematic for several reasons:
Also, some events have evolved since their first implementation, and they do not reflect their initial name anymore.
For example, see
EventProviderOpened
as of the writing of this issue.How will this feature be useful to you and others?
This is an internal change, which aims to improve the DX and the overall code architecture. No impact on UX is expected.
The feature request aims to streamline the Event Manager in several aspects.
Firstly, split the event definitions into meaningful categories (e.g.
events_internal.hpp
,events_achievements.hpp
, etc.) so that code is more modular and can be easily adapted to fit new needs (like new categories). This would also avoid a clog-up of the current large file that contains all the definitions.Secondly, use this request as leverage to carefully re-read, analyse, and possibly refactor some events. As stated in the section above, some events don't fit their initial names anymore, or have completely derived from their initial goal.
Lastly, it would be useful to have short but meaningful descriptions of the events in code comments above their respective definitions. Something that new contributors might struggle with is knowing what is expected to be passed in events when
post
ing them, so describing what exactly those parameters serve could help reading and understanding ImHex's architecture. This will lengthen the files, but I think it's for the greater good in the long run.Request Type
Additional context?
Assignee: @BioTheWolff (myself)
The text was updated successfully, but these errors were encountered: