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

Make Equip Swap work on Pause Menu with 'Any Cursor' Enhancement #4052

Merged

Conversation

inspectredc
Copy link
Contributor

@inspectredc inspectredc commented Apr 12, 2024

@inspectredc inspectredc changed the title Make Equip Swap work on Pause Menu with 'Any Cursor' Enhancment Make Equip Swap work on Pause Menu with 'Any Cursor' Enhancement Apr 12, 2024
@briaguya-ai
Copy link
Contributor

i know this is a new feature but really it feels like a bugfix - mind making a PR pointing to macready for it?

@inspectredc
Copy link
Contributor Author

Cool! I was gonna ask if i could point this to macready cause I want it sooner haha

@inspectredc inspectredc force-pushed the EquipSwap-AnyCursor branch from d972c9f to cf657aa Compare April 12, 2024 15:46
@inspectredc inspectredc changed the base branch from develop to develop-macready April 12, 2024 15:46
@inspectredc
Copy link
Contributor Author

Now pointing towards macready 👍🏼

Copy link
Contributor

@briaguya-ai briaguya-ai left a comment

Choose a reason for hiding this comment

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

it'd be great to have a few people pull down test builds and try out a few swaps, but that's definitely not required for us to :shipit:

@inspectredc
Copy link
Contributor Author

Oh yea, I have a video showcasing here too if that helps!

EquipSwap.mp4

@Malkierian
Copy link
Contributor

WTF, how'd you figure out it was such a miniscule change?

@inspectredc
Copy link
Contributor Author

inspectredc commented Apr 12, 2024

I think I always knew the issue was basically that the evaluation of 'can we move to any slot' was the issue when moving from the Z/R buttons. To quote my own explanation in the race-chat

"basically what happens is that it has always correctly gone from the Z/R buttons to the correct item, but the up input then causes it to go 1 up from there. the way equip swap works is that itll only update the current hovered item as long as the movement of the cursor was evaluated as a success". "since left/right inputs are done before up/down inputs the last state of the movement ends up being a fail, except with the enhancement where it allows it to succesfully move up"

then also the reason why this is frame perfect is that the hovered item gets set to 0 when on the Z/R buttons, but only after the evaluation occurs the very first time. there's a special case to basically ignore up/down inputs when the hovered item is 0 (note that I believe we dont update the hovered item to 0 when we move over empty slots, potentially allowing for a special new kinda equip swap I might investigate..) (looks like it does update when not starting from the Z/R buttons)

@Malkierian
Copy link
Contributor

Confirmed working on my end.

@Caladius
Copy link
Contributor

Not that I'm looking to add work but would it be beneficial to remove the Cursor on any Slot option and default it to Only in Rando? Since the player would no longer need to toggle it for Equip Swap, I'm unsure if there's any reason to set it to Never anymore

@Malkierian
Copy link
Contributor

Could it still be useful in vanilla speedruns? If so, we wouldn't want to remove the option completely. Changing the default, if it's not Only In Rando, would probably be good, though.

@Malkierian
Copy link
Contributor

Unless you meant to say "remove the Cursor On Any Slot Never option".

@Caladius
Copy link
Contributor

Could it still be useful in vanilla speedruns? If so, we wouldn't want to remove the option completely. Changing the default, if it's not Only In Rando, would probably be good, though.

Maybe? Not really sure on that. More of a "Can we use this to clean up the UI options somehow in a future PR"

@inspectredc
Copy link
Contributor Author

I personally can still see the reason why someone might want to have the only in rando option exist, for example, if they want to do speedruns on a vanilla file, but also still play rando. Maybe once we have better presets it can be removed

@Malkierian
Copy link
Contributor

If we think there's no reason to have it off in rando, though, we could just have it always on in rando in code, and have it be a toggle for applying to vanilla?

@Malkierian
Copy link
Contributor

Still probably something for a later PR, though.

@inspectredc
Copy link
Contributor Author

I think with rando v3 we're generally moving away from IS_RANDO without a cvar anyway (or smth to that effect)

@Caladius
Copy link
Contributor

100%, sorry if that wasnt apparent in my intentions. Not for this PR

@briaguya-ai briaguya-ai merged commit bfe1390 into HarbourMasters:develop-macready Apr 19, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants