• Hey Guest! Ever feel like entering a Game Jam, but the time limit is always too much pressure? We get it... You lead a hectic life and dedicating 3 whole days to make a game just doesn't work for you! So, why not enter the GMC SLOW JAM? Take your time! Kick back and make your game over 4 months! Interested? Then just click here!

Legacy GM [GM 8.1] What is wrong with this code? I can't seem to find the problem.

Momoro

Member
Hello :)

I work on my projects in GameMaker 8.1, because I prefer to use it and I do not currently own GameMaker: Studio 2.

I've put together a script that is supposed to be put into the Step event of an object (the player), and it works unless multiple keys are pressed,

Code:
if (keyboard_check_pressed(vk_up))
{
    if (global.goingDown != true && global.goingRight != true && global.goingLeft != true)
    {
        global.goingUp = true;
  
        sprite_index = spr_rpg_boy_walk_up;
  
        image_speed = 3;
  
        motion_set(90,3);
    }
}
else if (keyboard_check_pressed(vk_down))
{
    if (global.goingUp != true && global.goingRight != true && global.goingLeft != true)
    {
        global.goingDown = true;
      
        sprite_index = spr_rpg_boy_walk_down;
      
        image_speed = 3;
      
        motion_set(270,3);
    }
}
else if (keyboard_check_pressed(vk_right))
{
    if (global.goingUp != true && global.goingDown != true && global.goingLeft != true)
    {
        global.goingRight = true;
      
        sprite_index = spr_rpg_boy_walk_right;
      
        image_speed = 3;
      
        motion_set(0,3);
    }
}
else if (keyboard_check_pressed(vk_left))
{
    if (global.goingUp != true && global.goingDown != true && global.goingRight != true)
    {
        global.goingLeft = true;
      
        sprite_index = spr_rpg_boy_walk_left;
      
        image_speed = 3;
      
        motion_set(180,3);
    }
}
else if (keyboard_check_released(vk_up))
{
    global.goingUp = false;
  
    if (keyboard_check_pressed(vk_down))
    {
        global.goingDown = true;
      
        sprite_index = spr_rpg_boy_walk_down;
      
        image_speed = 3;
      
        motion_set(270,3);
    }
    else if (keyboard_check_pressed(vk_right))
    {
        global.goingRight = true;
      
        sprite_index = spr_rpg_boy_walk_right;
      
        image_speed = 3;
      
        motion_set(0,3);
    }
    else if (keyboard_check_pressed(vk_left))
    {
        global.goingLeft = true;
      
        sprite_index = spr_rpg_boy_walk_left;
      
        image_speed = 3;
      
        motion_set(180,3);
    }
    else
    {
        sprite_index = spr_rpg_boy_up;
      
        image_speed = 3;
      
        motion_set(270,0);
    }
}
else if (keyboard_check_released(vk_down))
{
    global.goingDown = false;
  
    if (keyboard_check_pressed(vk_up))
    {
        global.goingUp = true;
      
        sprite_index = spr_rpg_boy_walk_up;
      
        image_speed = 3;
      
        motion_set(90,3);
    }
    else if (keyboard_check_pressed(vk_right))
    {
        global.goingRight = true;
      
        sprite_index = spr_rpg_boy_walk_right;
      
        image_speed = 3;
      
        motion_set(0,3);
    }
    else if (keyboard_check_pressed(vk_left))
    {
        global.goingLeft = true;
      
        sprite_index = spr_rpg_boy_walk_left;
      
        image_speed = 3;
      
        motion_set(180,3);
    }
    else
    {
        sprite_index = spr_rpg_boy_1;
      
        image_speed = 3;
      
        motion_set(270,0);
    }
}
else if (keyboard_check_released(vk_right))
{
    global.goingRight = false;
  
    if (keyboard_check_pressed(vk_down))
    {
        global.goingDown = true;
      
        sprite_index = spr_rpg_boy_walk_down;
      
        image_speed = 3;
      
        motion_set(270,3);
    }
    else if (keyboard_check_pressed(vk_up))
    {
        global.goingUp = true;
      
        sprite_index = spr_rpg_boy_walk_up;
      
        image_speed = 3;
      
        motion_set(90,3);
    }
    else if (keyboard_check_pressed(vk_left))
    {
        global.goingLeft = true;
      
        sprite_index = spr_rpg_boy_walk_left;
      
        image_speed = 3;
      
        motion_set(180,3);
    }
    else
    {
        sprite_index = spr_rpg_boy_right;
      
        image_speed = 3;
      
        motion_set(270,0);
    }
}
else if (keyboard_check_released(vk_left))
{
    global.goingLeft = false;
  
    if (keyboard_check_pressed(vk_down))
    {
        global.goingDown = true;
      
        sprite_index = spr_rpg_boy_walk_down;
      
        image_speed = 3;
      
        motion_set(270,3);
    }
    else if (keyboard_check_pressed(vk_right))
    {
        global.goingRight = true;
      
        sprite_index = spr_rpg_boy_walk_right;
      
        image_speed = 3;
      
        motion_set(0,3);
    }
    else if (keyboard_check_pressed(vk_up))
    {
        global.goingUp = true;
      
        sprite_index = spr_rpg_boy_walk_up;
      
        image_speed = 3;
      
        motion_set(90,3);
    }
    else
    {
        sprite_index = spr_rpg_boy_left;
      
        image_speed = 3;
      
        motion_set(270,0);
    }
}
..which I'd thought I took care of inside of the code?

The global variables are set to false when the player is first created.

So, if I press Up, Down, Left or Right separately, it works fine, but if I press them simultaneously, it glitches and it does one of these two glitches every time:

1) It will not let the player move the object unless you figure out which key went wrong, which is different in almost every case.

2) The object will not stop moving, even if the player releases the correct key, until they press and/or release another key simultaneously.

Could anyone correct my code? I'm not seeing the problem, but I'm sure one of you can / do :)

Thanks for your time!
 

FrostyCat

Redemption Seeker
The code on this topic has basically the same problem as the code from this other topic. Each of these input actions completely negates the effect of all others without ever letting them combine, and that's a common rookie problem. The code should be replaced with an alternative that allows input to be combined, such as this:

Step:
Code:
var dx = keyboard_check(vk_right)-keyboard_check(vk_left);
var dy = keyboard_check(vk_down)-keyboard_check(vk_up);
if (dx != 0 || dy != 0) {
  direction = point_direction(0, 0, dx, dy);
  speed = 5;
} else {
  speed = 0;
}
The next time you do 8-directional movement or anything else where multiple influences cross-interact, do not code it in a way where a single influence can completely override all others. Let them cross-interact first, then operate on the net result.
 

Momoro

Member
Um, how do I change the sprite depending on which key is pressed, e.g. up/down left/right? This new shorter version of code is great, but confusing o_O Thank you!
 
Z

zendraw

Guest
you can just do an IF statement again. that code is basicly math.
when you press vk_right on your keyboard, this -> keyboard_check(vk_right) becomes 1, otherwise its 0.
so in that code,
var dx = keyboard_check(vk_right)-keyboard_check(vk_left);

when you press right it will look like this var dx=1-0; which is equal to 1;
if you press left it will look like this var dx=0-1; which is -1;
so when pressing right you get a positive value and when you press left you get a negative value, otherwise the value is 0;

so this is what you can do.
if (dx!=0)
{
if (dx>0) {set sprite for right};
if (dx<0) {set sprite for left};
} else
{
if (dy>0) {set sprite for down};
if (dy<0) {set sprite for up};
}

so it will set vertical sprites only when you are not pressing keys for horizontal movement.
 

Momoro

Member
you can just do an IF statement again. that code is basicly math.
when you press vk_right on your keyboard, this -> keyboard_check(vk_right) becomes 1, otherwise its 0.
so in that code,
var dx = keyboard_check(vk_right)-keyboard_check(vk_left);

when you press right it will look like this var dx=1-0; which is equal to 1;
if you press left it will look like this var dx=0-1; which is -1;
so when pressing right you get a positive value and when you press left you get a negative value, otherwise the value is 0;

so this is what you can do.
if (dx!=0)
{
if (dx>0) {set sprite for right};
if (dx<0) {set sprite for left};
} else
{
if (dy>0) {set sprite for down};
if (dy<0) {set sprite for up};
}

so it will set vertical sprites only when you are not pressing keys for horizontal movement.
Does this look right? (It works, but I'm looking for a second opinion):

GML:
image_speed = .2;

var dx;
dx = keyboard_check(vk_right)-keyboard_check(vk_left);
var dy;
dy = keyboard_check(vk_down)-keyboard_check(vk_up);

if (dx!=0)
{
if (dx>0) { sprite_index = spr_rpg_boy_walk_right; };
if (dx<0) { sprite_index = spr_rpg_boy_walk_left; };
} else
{
if (dy>0) {  sprite_index = spr_rpg_boy_walk_down; };
if (dy<0) {  sprite_index = spr_rpg_boy_walk_up; };
}

if (dx != 0 || dy != 0) {
  direction = point_direction(0, 0, dx, dy);
  speed = 5;
} else {

  if (sprite_index == spr_rpg_boy_walk_right)
  {
      sprite_index = spr_rpg_boy_right;
  }
  else if (sprite_index == spr_rpg_boy_walk_left)
  {
      sprite_index = spr_rpg_boy_left;
  }
  else if (sprite_index == spr_rpg_boy_walk_down)
  {
      sprite_index = spr_rpg_boy_1;
  }
  else if (sprite_index == spr_rpg_boy_walk_up)
  {
      sprite_index = spr_rpg_boy_up;
  }
 
  speed = 0;
}
That's what I came up with from what you both posted :) Thanks!
 
Z

zendraw

Guest
dont do else if. keeping statements separate is better
if (sprite index== walk right) {sprite index = boy right};
if (sprite index== walk left) {sprite index = boy left};

otherwise yes its right.
 

TheouAegis

Member
Actually in these cases it is far worse to keep the condition separate. Left and right are contradictory in this scenario, therefore you want an else. And in one case a switch will be cleaner also.

Code:
if (dx>0) { sprite_index = spr_rpg_boy_walk_right; };
if (dx<0) { sprite_index = spr_rpg_boy_walk_left; };
On this block (and similarly the vertical block below it), you want an else instead of if dx<0, since the second conditional is actually implied if the first conditional fails. therefore, there's no need to check the second conditional if you have an else.

Code:
} else  ⟨⟨⟨⟨
{
if (dy>0) {  sprite_index = spr_rpg_boy_walk_down; };
if (dy<0) {  sprite_index = spr_rpg_boy_walk_up; };
}
However, this part here will be handled differently depending on if you have an else or not. Very differently. If up and right are held at the same time, the Sprite will always face right because the conditional prevents the vertical Sprite change from running if the horizontal Sprite is set. However, if you remove that else, then both the horizontal Sprite change will run and the vertical Sprite change, but the vertical Sprite change will run last and therefore override the horizontal one, so the Sprite will always face up when holding up and right. Similarly, if you handle the vertical Sprite change first, and then had an else before the horizontal Sprite change, then the opposite would happen. similarly, if you had the vertical Sprite change first and the horizontal Sprite change afterward without an else in between, the horizontal Sprite change would override the vertical Sprite change.

Code:
if (sprite_index == spr_rpg_boy_walk_right)  {      sprite_index = spr_rpg_boy_right;  }  
else if (sprite_index == spr_rpg_boy_walk_left)  {      sprite_index = spr_rpg_boy_left;  }  
else if (sprite_index == spr_rpg_boy_walk_down)  {      sprite_index = spr_rpg_boy_1;  }  
else if (sprite_index == spr_rpg_boy_walk_up)  {      sprite_index = spr_rpg_boy_up;  }
Here, a switch would be better, but else-if is fine. You use else to avoid repeteitive, pointless code. If the Sprite is spr_rpg_boy_walk_right, you absolutely do not want to check any of the other sprites.
 
Last edited:
Z

zendraw

Guest
@TheouAegis you are wrong. in hur case with the sprites only one state will aways be positive, so doing if else if is stupid to say the least. a switch wuld be most suiting but since shes new its better to get used to if statements. so my example is perfectly fine

@FrostyCat again you dont grasp the situation and just see an oppurtunity to patronise. the code i wrote has nothing to do with the code you link. we dont check for the opposite here and restrain yourself from callign someone a rookie, it seems to me you know only books.
if (a==1) {};
if (a==2) {};

if (b) {};
if (!b) {};

can you spot the diffrence here? surely a profesional veteran like you can see the diffrence.

and as i said, a switch wuld be best, but its better for hur to get used to if`s first which shell be using alot more then switches.
 
Z

zendraw

Guest
It's better to get used to using ifs properly, including proper use of else in conjunction with if when it makes sense.

Can you spot the difference? Every bit of advice you've given here is bad.
who even are you and what experience do you have in programming and development? sayng my advice is bad is like sayng water is bad becouse coca cola is more tasty. id suggest if you dont have anythign useful to say or to react adequately and not abuse the like system becouse of bias, to find some other activity.
 

Nocturne

Friendly Tyrant
Forum Staff
Admin
@zendraw: First, please don't assume the OP identifies as female just because they have a female in their avatar and if you check their profile they have "male" flagged as well... Out of respect you should use they/them until otherwise indicated. Second, if someone replies with something you don't like and it's unrelated to the topic, just report it and move on please. Oh, and please stop writing "hur" and "wuld" and stuff... it's hard to take you seriously...

@Nidoking: If you have nothing helpful to offer to the topic then please refrain from posting... ;)

@Momoro: As you can see from this topic there are multiple ways to achieve the same goal in GameMaker (and programming in general). There also isn't always a "correct" way to do something and often just the fact that your code works and does the job is enough to move on! That said, I live by a general rule of thumb which is if a line of code or a variable check repeats more than twice then it can probably be done better, and in this case I'd consider reading the link that @FrostyCat gave and pay particular attention to the alternative given that uses the "switch" command. (or, since the code works for you, clap your hands and move on to the next thing! :p)
 

Nidoking

Member
I happen to think I'm contributing by helping the original poster, who evidently needs at least some amount of advice, determine which advice presented is good and which advice is terrible. Having the skill to distinguish between the two is right up there with not needing the advice at all. This is the benefit I can offer from nearly forty years of programming experience. I don't question the credentials of anyone whose posts answer the question already.

If you insist on my being more directly constructive, it leaps out at me immediately that there are checks for dx != 0 and, indirectly, dy != 0, and then a check right after that for dx != 0 || dy != 0. I see how the logic is meant to flow, but that kind of redundant check usually means that it can be simplified. With proper use of else, you can check for dx > 0, else dx < 0, and if neither is true, check dy > 0, then dy < 0. If all are false, then both values are zero and you handle the sprite change for stopping movement. You could even set direction and speed before checking the inputs, and if all inputs are zero, just set speed to zero. Given that you're moving along diagonals at integer speed, you might also need to round the final position when you stop, but that could just not have been posted here.
 

TheouAegis

Member
if (a==1) {};
if (a==2) {};

if (b) {};
if (!b) {};
Spot the difference? They are both bad on so many levels. If you set a to 2 when a is 1, it will run the second conditional immediately. If that's what you want, good, but in most cases you want them to be exclusive. If you set b to false when b is true, same thing. But neither pairing is mutual exclusive. Just because a is 1 doesn't mean a can't be 2; just because b is true doesn't mean b can't be false. Again, in some cases, you might want both conditions to run when they are both true and the same step, but most people actually do not.

And then there's the issue I mentioned before, which is one condition is going to override the other one, which is not always desirable. If you want to avoid one condition overwriting another, you need to use else. In the OP's code, the only part where this actually has any real bearing, is completely optional, which I explained.

And the third problem with your example, which I also mentioned in my post, is in the OP's case, your code is redundant. Consider if the very last part which I said we better as a switch is equivalent to your a==1;a==2 example. By the bOP's code, hey we'll never be 1 and 2 at the same time . It could be 1 and 5 at the same time, but not 1 and 2. Therefore, it is better to stop checking the value of a once you actually find a condition that is true. if a is 1, then you don't need to keep checking if a is 2 and then if a is 3 and then if a is 4.

And going back to again the OP's post, if dx is greater than 0, dx will not be less than 0, so there is no point in checking that condition. Likewise, since both of those conditions are inside of another condition that checks if dx is non-zero, there is absolutely no point in even checking if dx is less than zero, because it is fundamentally implied that if dx is not greater than 0, dx must be less than 0. It's basic logic.
 
Z

zendraw

Guest
first of all, it all depends on context, context here being the sprites in questions are never the sprites that she sets. thats why those values have names and are not displayed as numbers, so you can know what your coding, whats the context. knowing a will never be to what you set you can do whatever yhou want.
not long ago i posted a video from a programmer who condemns this notion that your code has to be universal and perfect, it cant be, and if you try to make it youll never get anything done and make it so complex it will take you forever to make anything.

and i think you dont get the context of my post, and that example was for frosty. his post was utterly unadequate and out of place and not understanding the code or acts as if he doesnt understand it.
his example
if (b) {};
if (!b) {};
has nothing to do with mine
if (a==1) {};
if (a==2) {};

even without context.

all your doing is explaining in what conditions that code is bad. so whats your point exactly? your not adding anything productive here, nor does frosty. your just patronising. every code can be bad depending the context. in this situation the code i suggest and the code momo has is perfectly fine.
 

Momoro

Member
Wow.. this thread got heated 😅

I appreciate all of your advice, whether it was good or not- the fact that you're trying to help is what's important to me :)
 

Nidoking

Member
all your doing is explaining in what conditions that code is bad
your not adding anything productive here
Ah, I see! You've provided examples of mutually contradictory statements to illustrate one of the rare situations when your construction is not distinctly less good than other, almost always better ways to do things.

every code can be bad depending the context
This is technically true, I suppose. However, some things are bad in any context, and some things are only bad if you go out of your way to devise a context that doesn't relate to anything that anyone would ever want to do. Any novice programmer who takes anything you've said in this thread seriously will be worse off for having read it. The few things you've said that aren't simply completely wrong are, at best, edge cases that will confuse anyone attempting to learn how to do boolean logic correctly. You don't start learning a language by concentrating on exceptions to the rules that work in 99.95% of all situations. Learning good habits early leads to better skill overall, while learning bad habits leads to frustration and having hundreds of posts here simply titled "HELP ME" and all ending with the same simple problem.

I'm not saying you can't use if without else. I'm saying that if you don't know how else works, learn how else works before you start using if. Then you can decide whether what you're doing is the .05% case where using else is wrong, and you'll be correct in your judgment. Of course code isn't going to be perfect, but why should that mean anyone should go out of their way to make it intentionally bad?
 
S

SSJCoder

Guest
if you want to give advice to the OP please DM them @Nidoking

if you'd like to create a post about "bad" and "good" code I'm sure there is a forum section for that, going into topics and just making a bunch of posts about this will probably just create chaos (especially if you are calling people out), instead of actually helping anyone ..

not discrediting your advice
 

Amon

Member
@Nidoking is a very experienced coder and has an exceptional knowledge and technical prowess in game programming logic. I for one have only ever seen him offer constructive, correct advice and offers solutions and explanations that make those solutions understandable. Also, he has never put his knowledge and experience on this forum in a way that would be deemed demeaning and an attack at said person's lack of skill.
 

TheouAegis

Member
all your doing is explaining in what conditions that code is bad. so whats your point exactly? your not adding anything productive here, nor does frosty. your just patronising. every code can be bad depending the context. in this situation the code i suggest and the code momo has is perfectly fine.
Sorry for misconstruing the a & b part.

I never said momo's code isn't perfectly fine, nor did I say your suggestions and even your argument with Frosty were completely wrong. Even my switch suggestion was just that to momo -- a suggestion. In both my posts, I explicitly stated there is a time and place for not using else, as well as a time and place for not using else-if. And in both posts I explained the logic behind if-if, if-else, and if-else-if, and why in this situation if-if is the worst method. The dx> advice was geared toward speeding up momo's code and teaching Philosophy 104. The dx-else-dy advice was geared toward explaining to momo how the inclusion of else affects that code, how the exclusion of else affects that code, and how the order of that code affects the code. And I even explicitly said if-else-if at the end of momo's code was actually better than your suggestion to not use else. Your suggestion required n conditions, momo's required (1,n) conditions, and a switch would have trimmed it down further.

And telling someone to not learn about switches because they are new
a switch wuld be most suiting but since shes new its better to get used to if statements
This isn't Japanese, where you should learn your kana before your kanji, this is programming. And since a switch is even more flexible than if-else-if, or even if-if, it should be taught when the moment arises when the student has already demonstrated an understanding of if and else, which momo has done.

If you want more constructive criticism of your post, here:
Code:
if sprite_index==spr_run_right {sprite_index = spr_walk_right;}
else if sprite_index==spr_run_left {sprite_index = spr_walk_left;}
else if sprite_index==spr_run_up {sprite_index = spr_walk_up;}
else if sprite_index==spr_run_down {sprite_index = spr_walk_down;}
In this code, if the sprite is spr_run_right, the program will stop immediately and only one sprite will be checked. If the sprite is spr_run_left, two sprites will be checked before it stops. And in each case, sprite_index will need to be loaded into the register for each check.
Code:
if sprite_index==spr_run_right {sprite_index = spr_walk_right;}
 if sprite_index==spr_run_left {sprite_index = spr_walk_left;}
 if sprite_index==spr_run_up {sprite_index = spr_walk_up;}
 if sprite_index==spr_run_down {sprite_index = spr_walk_down;}
By removing the else, you force the program to check all 4 sprites every time. Instead of having a 33% chance of slowdown, you now have a 100% chance of slowdown. It's minor slowdown in this situation, but it's there and could be exacerbated by in other situations.
 
Z

zendraw

Guest
i am aware of the performance issue but i see optimisation as a finishing task, rather then developing issue. i pretty much never stack ifs one on top of another but thats becouse i alredy know how to do stuff. the reason for my backlash is becouse i approach noob questions in a sense that they have to experiment with what they have, and you can experiment alot with just if else. anyway for me it is resolved.
@Momoro here is how this is done typically, so you can be more confused.
GML:
var dir=floor((point_direction(0, 0, dx, dy)+45)/90);

if ((dx!=0) or (dy!=0))
{
    sprite_index=spr_run0+dir;
} else
{
    sprite_index=spr_idle0+dir;
}
@Nidoking you are just projecting and turning this topic in a drama.
 

TheouAegis

Member
@Momoro here is how this is done typically, so you can be more confused.
GML:
var dir=floor((point_direction(0, 0, dx, dy)+45)/90);

if ((dx!=0) or (dy!=0))
{
    sprite_index=spr_run0+dir;
} else
{
    sprite_index=spr_idle0+dir;
}
Ooooh careful. As much as I love that code, you're going to get soooo much flack for even bringing up referencing asset indexes indirectly like that in Game Maker. I'm on a lot of people's s***lists for pushing that back when it actually worked in GM (prior to GMS2.3). 🤪
 

Nidoking

Member
you can aways just put the assets in arrays for this approach.
Wow! This is actually a really good way to do this. So... why did you post what you posted instead of posting this, specifically with the intent to confuse the poor OP who is earnestly trying to learn?
 

TheouAegis

Member
you can aways just put the assets in arrays for this approach.
Yes, which I was going to say in my last reply because that's (somewhat) encouraged, but you didn't say to use arrays, you said to use the asset indexes explicitly, which doesn't even work in GMS2.3 for whatever reason they came up with. 👾 Although it's only from 2.3 onward....
 
Z

zendraw

Guest
becouse momos new,he/she may not even know what or how an array is, and as alredy stated its better to get a job done then being barraged with tons of information.

there are 2 stages for me in learning anything.
stage1- you become familiar and accustomed or whtever in the activity, essentially you need only enough knowledge so you can do the thing your own way.
stage2- since now you have a way of doing things, you can easily add more knowledge to your arsenal and actually understand the knowledge becouse now you are aware of whats it all about. and you can expand to infinity now without a problem.

so basicly how league of legends have established the engine of the game, now they dont have a problem expanding it with new champions or maps til infinity or theyr creativity allows it, or shuld i say the creativity of others and the law allows it...
 
Top