Rework ActorViewer to use hooks#5474
Conversation
| }, | ||
| "bgCheckFlags"); | ||
|
|
||
| if (Button("Refresh", ButtonOptions().Color(THEME_COLOR))) { |
There was a problem hiding this comment.
Calling this out since the whitespace changes hide this, but I removed this button. It didn't do anything useful than refresh the actor list, which is done a different way now. This also removes the need of the retrieval method.
soh/src/code/z_actor.c
Outdated
| Actor* newHead; | ||
| ActorDBEntry* dbEntry; | ||
|
|
||
| GameInteractor_ExecuteOnActorDelete(actor); |
There was a problem hiding this comment.
I was unsure where to put this in the function, but I chose right at the start because, since this function destroys and eventually frees the actor, it make sense for hooks to respond to it with as much information about it as possible.
| ImGui::SameLine(); | ||
| DrawGroupWithBorder( | ||
| [&]() { | ||
| if (display != nullptr) { |
There was a problem hiding this comment.
I'm now hiding the actor information until one is actually selected. I always disliked the "dummy" actor solution, because it threw me off that it wasn't an actual actor in the world, and that it was somehow "Link" in the "Switch" category...
| void ActorViewerWindow::InitElement() { | ||
| ResetData(); | ||
|
|
||
| GameInteractor::Instance->RegisterGameHook<GameInteractor::OnActorInit>([](void* refActor) { |
There was a problem hiding this comment.
I think you need to keep track of hook ids returned here & unregister them in ResetData
(assuming InitElement can be invoked multiple times, otherwise no need to call ResetData)
There was a problem hiding this comment.
Well, no window InitElement is ever called more than once, so that's not an issue, I would tend to agree that ResetData doesn't need to be called here, unless those aren't the default states of those variables.
There was a problem hiding this comment.
I went ahead and removed it. It was kinda leftover from when there was more state that had to be reset. We can just rely on default initialization.
As for the hook ids, I do agree we are leaking them, but this is a singleton whose lifetime is the lifetime of the entire application, so I didn't worry about it (and the existing hook was already being leaked). Although, it does make me want to make a RAII hook manager...
* Rework ActorViewer to use hooks * Rework ActorViewer to use hooks * Remove ResetData
The current Actor Viewer currently has several situations that can lead or crashes or reading freed memory, such as when you are viewing an actor that dies, or when you reload a scene. The Actor Viewer tries to know when to reload or invalidate its data, but there is no good way to catch all circumstances where this happens. Luckily, hooks are the perfect solution to this.
This PR reworks the Actor Viewer to listen to hooks and update its state that way. This needed several new hooks, but I think they capture important events that were overdue for hooks anyways. I also took the time to rework the internal state to simplify it, now that the state was moving from a bunch of static function variables to class variables.
Build Artifacts