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

MemoryWidget: Make search address a combobox that holds address history. #11185

Merged

Conversation

TryTwo
Copy link
Contributor

@TryTwo TryTwo commented Oct 21, 2022

Always update the combobox when a new target address is sent.

The main problem was that navigating to an address from another widget wouldn't update the address box. It worked for peeking at a section, then going back to the original, unchanged search address, but not much else. I added a 10 slot history of addresses so you could keep your old address. All navigated to addresses (from other widgets) will go into the text box and history box automatically and the original address that gets overwritten will be saved in the combo box too.

Two issues in doing this were the offset box and typed addresses. The offset box is too cumbersome to worry about imo, selecting an address from the combobox will add the offset box to it. Typed addresses don't feel like they should immediately be added to the history box. If you press ENTER it will go into the history.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When m_search_address->maxCount is reached, entries won't be inserted anymore. Is that an intended behaviour (i.e. to only save the first 10 entries)?

Otherwise, the code LGTM.

@TryTwo
Copy link
Contributor Author

TryTwo commented Oct 30, 2022

Oh thanks for pointing that out. It needs to push out the oldest when it hits max.

Always update the combobox when a new target address is sent.
@TryTwo TryTwo force-pushed the PR_MemoryWidget_Address_Input_History branch from d413184 to 053320b Compare October 30, 2022 05:42
@TryTwo
Copy link
Contributor Author

TryTwo commented Oct 30, 2022

I just changed it to m_search_address->setMaxVisibleItems(8); which should be fine. It'll just give a scrollbar.

Copy link
Contributor

@sepalani sepalani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

One future improvement would be the ability to delete entries from the history. Something similar to Chrome/Firefox where you could delete the autocomplete/autofill entry by selecting it and pressing the del key.

@JMC47 JMC47 merged commit 950e1f9 into dolphin-emu:master Oct 30, 2022
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants