Continous Movement for SDL 2 Joysticks?

Started by
38 comments, last by Alberth 6 years, 10 months ago

Good find!

Instead of just fixing the error, fix your compiler settings first to make it scream in agony when it finds assignments in conditions.

I went through some event code too, and unlike Lactose, I didn't find anything major related to the joystick code, however


case SDLK_UP:
    if (this->getPlayerRectPosY() == 0)
        pVelY = PLAYER_VELOCITY_Y;
    pVelY = -PLAYER_VELOCITY_Y; break;

You are aware that your "if" statement isn't doing much here, right?

Also, I'd recommend putting the "break" on a line by itself.

Advertisement

else if (e.type = SDL_KEYUP)

This is always true.

Double equals, not single equals.

Bump up your warning level in your compiler/IDE.

As I thought could happen, I did something silly. Thanks for your time looking through the code.

Good find!

Instead of just fixing the error, fix your compiler settings first to make it scream in agony when it finds assignments in conditions.

I went through some event code too, and unlike Lactose, I didn't find anything major related to the joystick code, however


case SDLK_UP:
    if (this->getPlayerRectPosY() == 0)
        pVelY = PLAYER_VELOCITY_Y;
    pVelY = -PLAYER_VELOCITY_Y; break;

You are aware that your "if" statement isn't doing much here, right?

Also, I'd recommend putting the "break" on a line by itself.

Thank you for the heads up. That condition should be '<' instead of '==' so that the character is able to get to the edge of the screen. I have been putting the break statements on their own line, but I tried the same line. It seems I like different lines better, though. That is what is recommended overall anyways, right?

So, it seems like changing the '=' to '==' solved the problem. I will definitely check the warning settings on my compiler. I need to turn some of them off, actually. The one for this situation appeared, but I turned so many on that I thought I would check them all later! Some of them trigger warnings from include files or from things that are mandatory in SDL 2 (such as the main function) that seem superfluous.

On that note, -Wshadow triggers on the function parameter lists of handlePlayerEvents. Any thoughts?

That condition should be '<' instead of '==' so that the character is able to get to the edge of the screen.
Hmm yes indeed. However, I am more worried about the line below it, where you assign pVelY with a new value, no matter what the "if" assigned.

I have been putting the break statements on their own line, but I tried the same line. It seems I like different lines better, though. That is what is recommended overall anyways, right?
Pretty much everybody evolves to that style eventually, so I think there are some bigger advantages to it than just "everybody uses it". It might have to do with ease of reading the code. "The last statement on a line" takes more effort to find than "the last line of a block of code". The less effort goes into reading the code, the more effort you can put into solving the problem.

That condition should be '<' instead of '==' so that the character is able to get to the edge of the screen.
Hmm yes indeed. However, I am more worried about the line below it, where you assign pVelY with a new value, no matter what the "if" assigned.

I get what you are saying. You think having 2 breaks in one switch statement is good- one with the if (it handles collisions) and one underneath the if (if there are no collisions)? It's weird how it has been working already though, and I don't know how I didn't think of something like that already.

Ok, putting an extra break caused no problems. I have read that using break and continue statements (other than the one usually in a switch statement) should be avoided if possible, however. The code works this way too, though.

The advantage of an if/else is clarity, it's very obvious that I execute only one of the assignments then. Since the lines are practically next to each other, it's easy to see how both alternatives differ from each other.

The cost of an if/else is additional indentation of the "else" statements, and understanding that the code is executed when the condition at the "if" line does NOT hold. While that sounds easy enough, try it when you have 5 nested if/else statements, where each innermost alternative is 30-40 lines long. "I am at around line 340, now what was the condition for this code again? Let's look for that, 5 pages back".

The advantage of multiple exit points is that you do a case-by-case kind handling, "if foo holds, handle that case, and done." next, "if foo2 holds, handle that case, and done." It's easier to understand if "foo" and "foo2" (and "foo3" ...) are not much related. If they are closely related, you have the same problem as with the "else" case where it becomes tricky to understand when exactly the code is executed.

The second advantage of multiple exit points is less indentation. With 5 nested if/else statements, you're looking at 20 to 40 spaces before the first character of a statement. At some point you have more white-space than actual code.

The disadvantage of multiple exit points is lack of coherence. It's a long list of cases, and there is no telling how many cases there are, or whether you have done them all (if you missed one case, the last case is actually being used for 2 unrelated cases, without anything warning you for that).

Obviously, there are other ways to solve the above too, 5 nested if/else statements, each with 30 lines of handling code shouldn't be in one function in the first place. A better solution here can be to make a separate function for each alternative, and have the 5 nested if/else constructs call a function or two at the appropriate point.

You think having 2 breaks in one switch statement is good- one with ...
Above I tried to explain the advantages and disadvantages of the alternatives, as I see them. however, for this case, none of them apply, since the statements handling the alternatives are 1 line exactly. Personally, I'd go for the coherence of the if/else construct, but try them both, and see what you like best.

It's weird how it has been working already though, and I don't know how I didn't think of something like that already.
We are very bad at reading code that we think we know. That's how you and I missed the assignment in the condition, and that's how you and Lactose missed the if statement that does nothing.

Did you know that while reading normal text, we actually skip 50% of the letters? We look at start and finish of a word, and the general shape, and our brain fills in the missing letters. The same thing happens here, except that code has a much higher information density, and unlike spelling errors, single letter mistakes are deadly.

This topic is closed to new replies.

Advertisement