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

Rigidity slider no longer jumps to mouse #3452

Closed
wants to merge 2 commits into from
Closed

Rigidity slider no longer jumps to mouse #3452

wants to merge 2 commits into from

Conversation

Cowman9002
Copy link

@Cowman9002 Cowman9002 commented Jun 28, 2022

Brief Description of What This PR Does

The PR changes the rigidity slider so that once it becomes locked at an edge, resetting it through a undo will no longer cause it to snap to the position of the mouse when the mouse is hovered.

Related Issues

closes #1533

Progress Checklist

Note: before starting this checklist the PR should be marked as non-draft.

  • PR author has checked that this PR works as intended and doesn't
    break existing features:
    https://wiki.revolutionarygamesstudio.com/wiki/Testing_Checklist
    (this is important as to not waste the time of Thrive team
    members reviewing this PR)
  • Initial code review passed (this and further items should not be checked by the PR author)
  • Functionality is confirmed working by another person (see above checklist link)
  • Final code review is passed and code conforms to the
    styleguide.

Before merging all CI jobs should finish on this PR without errors, if
there are automatically detected style issues they should be fixed by
the PR author. Merging must follow our
styleguide.

@revolutionary-bot
Copy link

Thank you for contributing to Thrive.

Before your contribution can be accepted, you need to sign our CLA.
You can sign it, and find more info about it, here: https://dev.revolutionarygamesstudio.com/cla

Once your contribution has been accepted you can ask to be included in the credits (https://wiki.revolutionarygamesstudio.com/wiki/Team_Members) and you can apply to join the team and use this work as a sample: https://revolutionarygamesstudio.com/application/

@hhyyrylainen
Copy link
Member

I can't say I'm a big fan of working around an engine bug like this... Especially as this doesn't prevent the slider change events from actually being emitted (or at least I assume so). Does this even prevent the slider from moving without MP? I think that's a very important part to keep.

@Cowman9002
Copy link
Author

I'm not sure about the change events being emitted, but the slider is still unable to move without MP due to this code:

int cost = Math.Abs(rigidity - intRigidity) * costPerStep;

if (cost > Editor.MutationPoints)
{
    ...
    rigidity = intRigidity > rigidity ? intRigidity - stepsLeft : intRigidity + stepsLeft;
}

Which can be found in the file src\microbe_stage\editor\CellEditorComponent.cs starting on line 890.

@hhyyrylainen
Copy link
Member

That makes the code much more likely to have bugs, we aren't preventing the slider from attempting to move. Still at best this PR is a hack around an engine issue in my opinion.

@84634E1A607A
Copy link
Contributor

Once when I was tackling #3991 I tried to delete this code and make dynamic MP decide if the slider can still move (but failed). I prefer that we never disable the sliders but fix that logic mentioned above

but the slider is still unable to move without MP due to this code

@hhyyrylainen
Copy link
Member

Once when I was tackling #3991 I tried to delete this code and make dynamic MP decide if the slider can still move (but failed). I prefer that we never disable the sliders but fix that logic mentioned above

I suppose, yes, that'll actually make sense. We just need a way to prevent dragging it in a direction that would cause negative MP, but allow dragging in a direction that gives back MP.

@Cowman9002
Copy link
Author

I'm not sure if this is exactly what you mean, but I was also playing around with the code and found out that if you write:

int cost = (Math.Abs(rigidity) - Math.Abs(intRigidity)) * costPerStep;

The slider can not go any farther into its domain (positive to farther positive) but can still go back even to the opposite domain as long as the player has enough MP to do so.

I didn't put it in the PR because I didn't want to do any extra that what the issue reported.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Inactive slider doesn't detect mouse up
4 participants