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

DBM Event Register Needs Review #95

Open
MysticalOS opened this issue Apr 8, 2022 · 0 comments
Open

DBM Event Register Needs Review #95

MysticalOS opened this issue Apr 8, 2022 · 0 comments
Assignees
Labels
🐛 Bug Something isn't working help wanted Extra attention is needed Low Priority This issue will not be worked on unless there are no high or medium tasks left

Comments

@MysticalOS
Copy link
Member

MysticalOS commented Apr 8, 2022

The whole event manager probably needs complete rewriting, because registershorttermevents was a hacky addition and and apparently has actually been broken for years. Fortunately no mods currently seem affected by the breakage since the way it's broken doesn't affect way most mods register/unregister events. As such this task is mostly for tracking purposes for when this is eventually modularized.

Intent: DBM was supposed to support better registering and unregistering events at both core and mod level in short term. Meaning events don't stay registered all the time and wasting CPU.

Function: Register short term events is supposed to be stand alone from register events regular (which is intended mostly for events that stay registered all the time). Unregister short term events is supposed to only clear the short term events that were registered.

Actuality: Register short term events shoves short term events into same table as long term events. Unregister short term event that's also a long term event just nukes both. Fortunately there is no mod that actually does this (not even core thankfully)

Likely need: Because every part of event handler runs through a single table for registered events, probably still need a single table tracking what's registered (long or short term). However when long or short term events are actually registered THESE should probably each be in their own table. Then when unregistering short term events, We remove any events in short term table that aren't also in long term table from the main event tracker table.. Something like

if shortTermEvents[event] then
	if not longTermEvents[event] then
		shortTermEvents[event] = nil
		--run actual event unregister for event
		registeredEvents[event] = nil--Untrack from main tracked events table
	else
		shortTermevents[event] = nil--Only wipe from short term events, but not unregister it if it's still being used by long term events
	end
end
@MysticalOS MysticalOS added ⚠️ High Priority This issue makes DBM hard to use/non functional. Usually Core or GUI issue 🐛 Bug Something isn't working help wanted Extra attention is needed labels Apr 8, 2022
@MysticalOS MysticalOS added this to the Retail Patch 9.2 milestone Apr 8, 2022
@MysticalOS MysticalOS added Medium Priority This issue doesn't make DBM unusable, but makes it less useful until complete Low Priority This issue will not be worked on unless there are no high or medium tasks left and removed ⚠️ High Priority This issue makes DBM hard to use/non functional. Usually Core or GUI issue Medium Priority This issue doesn't make DBM unusable, but makes it less useful until complete labels Apr 9, 2022
@MysticalOS MysticalOS changed the title DBM Event Register is broken DBM Event Register Needs Review Apr 9, 2022
@MysticalOS MysticalOS removed this from the Retail Patch 9.2 milestone Apr 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working help wanted Extra attention is needed Low Priority This issue will not be worked on unless there are no high or medium tasks left
Projects
None yet
Development

No branches or pull requests

2 participants