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

Editor Scroll Speed small tweaks #1611

Merged
merged 3 commits into from
Jan 10, 2021
Merged

Conversation

Raoul1808
Copy link
Contributor

I wanted to push a small tweak I made to the editor when working on editor samples.
Basically you can now hold the Control and Shift key to scoll respectively slower (0.5x) and faster (3x).

The first commit has been tested on Windows and Ubuntu, but the second one only on Windows.
I consider this tweak done, it could be improved to allow creators to enter other scroll values (with a limit) or speed modifiers.

ev.key.keysym.mod & KMOD_SHIFT) {
m_scroll_speed = 96.0f;
}
else if (ev.type == SDL_KEYDOWN &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Using a nested if statement, you can avoid checking ev.type == SDL_KEYDOWN twice.

Copy link
Member

Choose a reason for hiding this comment

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

While you're changing this part of the code - please format your if statemets as:

if (condition)
{
  ...
}
else if (condition)
{
  ...
}
else
{
  ...
}

.. with all brackets of their own lines. Saves some code cleaning time :^)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't think of nested if statements, I'm working on it rn.
@Semphriss shouldn't I remove the brackets since the speed changes are one line only?

Copy link
Member

Choose a reason for hiding this comment

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

Usually, brackets are omitted when the operation is trivial and the body of the if statement is practically guaranteed to never need additions (like if (must_be_false) return;).

With if-else statements, it's even more important since it's more likely to cause trouble with maintenance later (imagine someone adding a line when you didn't use brackets). For the moment, I'd advise keeping the brackets even though it's only one line, but I'm open to discussion.

@tobbi
Copy link
Member

tobbi commented Jan 10, 2021

Is this ready to be merged?

@tobbi tobbi merged commit e99eafc into SuperTux:master Jan 10, 2021
@tobbi
Copy link
Member

tobbi commented Jan 10, 2021

According to @Semphriss this is ready to be merged. Thanks for this, @Raoul1808

@Raoul1808 Raoul1808 deleted the editor-scroll-speed branch January 13, 2021 17:47
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.

None yet

4 participants