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

CheatSearch: Automatically update Current Values #12263

Merged

Conversation

Dentomologist
Copy link
Contributor

CheatSearchWidgets keep track of two kinds of values: "Last Value" and "Current Value". There's a Last Value for each of the (potentially millions) of addresses that haven't been filtered out yet, which is the value that gets used by the "last value" filter option. For the (maximum of 1000) addresses that are shown in the table at any given time in the address table there is also a Current Value. At the moment Cheat Search only updates Current Values when the user either filters the current set of addresses or presses the "Refresh Current Values" button.

This PR makes the Current Values update automatically at the end of each frame, unless the user toggles a checkbox to restore the previous behavior.

To minimize the performance impact of this change several shortcuts are employed. While the game is running Current Values will only be updated while the Cheats Manager is open and the selected tab is a CheatSearchWidget. That CheatSearchWidget will be the only one updated, and furthermore only the currently visible rows will be updated. If the user scrolls the address table or switches to a different Cheat Search there will very briefly be some rows with outdated values, but this will be corrected by the next frame and the user probably won't even have time to notice the old values.

When the game is paused all Current Values in all CheatSearchWidgets are updated to prevent any such outdated values.

Specify the begin and end indices instead of filling a vector with all
the indices which are continuous anyway.
The table only needs to be recreated when the displayed addresses might
change. If we're just refreshing the current values then update those
table cells and leave the rest of the table alone.
@TryTwo
Copy link
Contributor

TryTwo commented Oct 31, 2023

I like the idea of waiting for the end of the frame to trigger the update. I've tried to do memory updates in memoryviewwidget and had some issues, especially with debugger windows being open. I also made a private build of the old cheat search with auto-updates that worked well.

Right now it's freezing everything when I try to run the first search. Even if there are 0 hits., Even if I pause, search, reset search, unpause. The game doesn't play. Debug UI is off.

@Dentomologist
Copy link
Contributor Author

Huh, the only freezing I've seen is the normal temporary freeze when you do a scan of the full standard address space (which is a lot longer on the debug builds I normally use), but it doesn't sound like that's what you're talking about. Having the debug UI on or off hasn't made a difference for me.

Can you list the full set of variables on the search you're using (address space, physical/virtual type, etc.)? Does checking/unchecking the "Automatically update Current Values" box have any effect? Does a fresh config change anything?

@TryTwo
Copy link
Contributor

TryTwo commented Oct 31, 2023

Hmm the moment I hit Automatically Update Current Values the emulation locks up. I can still interact with the screen. Unchecking the box does nothing. Trying to pause the game freezes stuff up even more.

Hmm on more testing, sometimes simply opening the cheat search window with Auto Update checked will cause the game to hang (this shouldn't be running any code at this point), didn't even run a search.

Starting and using cheat search with auto update off works fine.

Using default options: Typical, signed int 32, is equal to value 12. They actually don't matter, I mixed them up multiple times. Parse as hex/display hex doesn't matter. Tried in portable mode. Wii games. Windows platform.

It feels like it isn't releasing from the OnFrameEnd, but I haven't opened it in VS to check.

/edit If I enable Auto Update with no cheat search started, it pauses the game and break it, but literally changes the Play button to Pause.

@TryTwo
Copy link
Contributor

TryTwo commented Oct 31, 2023

Maybe download the buildbot version like I did and see if there's something weird with it?

@Dentomologist
Copy link
Contributor Author

When Dual Core is enabled the event is triggered from the video thread instead of the CPU Thread, which in turn incorrectly triggers PauseAndLock from not-the-host-thread. I'll try to figure out a better way to do this.

@TryTwo
Copy link
Contributor

TryTwo commented Oct 31, 2023

That's not intentional right? Could that be happening to other things as well?

@AdmiralCurtiss
Copy link
Contributor

AdmiralCurtiss commented Oct 31, 2023

AfterFrameEvent was added in #11522 to replace the monolith of the Renderer class, so yeah it running on the GPU thread makes sense. I'm not sure if we have an event to hook that runs once per frame on the CPU thread...

e: Yeah you'd have to add something in VideoInterfaceManager::OutputField(), that runs on the CPU thread. Doesn't appear like there's anything hookable there at the moment.

@TryTwo
Copy link
Contributor

TryTwo commented Oct 31, 2023

When I tried to do it, I did like every 100ms which worked for me. I'm not sure if AfterFameEvent would cause it to run at a certain/predictable place in code execution, which might be nice.

At the end of each frame automatically update the Current Value for
visible table rows in the selected and visible CheatSearchWidget (if
any). Also update all Current Values in all CheatSearchWidgets when the
State changes to Paused.

Only updating visible table rows serves to minimize the performance cost
of this feature. If the user scrolls to an un-updated cell it will
promptly be updated by either the next VIEndFieldEvent or the State
transitioning to Paused.
@Dentomologist
Copy link
Contributor Author

OK, dual core's working now. I used EndField instead of OutputField since it already calls Core::OnFrameEnd which in turn triggers the memory watcher (if it's enabled), so that's consistent.

I'm not sure if the comment I added in VideoEvents.h is the best way to describe the event, but I wanted to distinguish it from the other ones and particularly that it's on the CPU thread.

@TryTwo
Copy link
Contributor

TryTwo commented Nov 3, 2023

It generally works, but then something goes wrong. Spamming pause / play at certain points (maybe when the numbers are updating) causes the game to become incredibly slow or completely stop. Nothing else freezes, just the game stops advancing frames. Code widget suggested it was stuck somewhere it couldn't get out of with ps operations.

This happened multiple times, first with a normal rate of pausing and messing around. Happens with debug ui off. Does not happen when the memory area is unloaded, only after it gets values and presumably is being used.

@TryTwo
Copy link
Contributor

TryTwo commented Nov 8, 2023

Update: My previous issue is unrelated to this PR. Things are working good for me.

@iwubcode
Copy link
Contributor

iwubcode commented Nov 12, 2023

I haven't looked this over just yet but want to say this is great to finally see!! Can probably use this same idea in the watches window in the future.

@lioncash lioncash merged commit 731013c into dolphin-emu:master Nov 28, 2023
11 checks passed
@Dentomologist Dentomologist deleted the realtime_cheatsearch_update branch November 28, 2023 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants