YoYoGames / GameMaker-Bugs

Public tracking for GameMaker bugs
13 stars 5 forks source link

In-Game: draw_self() should allow being passed -1, rather than showing an error #2856

Closed backYard321 closed 6 months ago

backYard321 commented 7 months ago

Is your feature request related to a problem?

Sprite drawing functions like draw_self() and draw_sprite() will throw an error when the sprite index being referenced is -1. However, there are a lot of cases where an object won't have a sprite associated with it, but will still use this function, resulting in an error being thrown unexpectedly and your game quitting.

So, every time, you have to specify something like:

if sprite_exists(sprite_index)
{
    draw_self()
}

or

if sprite_index != -1
{
    draw_self()
}

in order to avoid your game erroring out. This is easy to forget because it's counterintuitive, and has caused a few problems both in my own games and playing games made by other people. I also think the benefit of this is questionable.

Describe the solution you'd like

In cases where the index number being referenced doesn't correspond to a sprite, simply don't perform the drawing function rather than trying to use the index and throwing an error. Or at least, don't throw an error in cases where the sprite is -1.

Describe alternatives you've considered

No response

Additional context

No response

gnysek commented 7 months ago

Yeah, many function were adjusted already to work with -1 or even undefined and not crash. So this one should be also affected.

backYard321 commented 7 months ago

Yeah, many function were adjusted already to work with -1 or even undefined and not crash. So this one should be also affected.

Definitely - similar to how with undefined, with noone, with -1 will just be skipped over, instead of throwing an error! IMO it makes coding much cleaner.

katsaii commented 7 months ago

draw_self should definitely not error if the sprite is not assigned.

However, this should not extend to draw_sprite and the other draw functions. It just opens the doors to more difficult to track down bugs, unless you want that?

Checking/not checking whether a sprite exists shows intent:

Let's not write difficult to debug code just to save a few keystrokes!

backYard321 commented 7 months ago

draw_self should definitely not error if the sprite is not assigned.

However, this should not extend to draw_sprite and the other draw functions. It just opens the doors to more difficult to track down bugs, unless you want that?

Checking/not checking whether a sprite exists shows intent:

* Checking whether a sprite exists shows that the sprite could be optional, and that you're accounting for this case.

* Not checking whether a sprite exists shows that the sprite is **NOT optional**, and in those cases you would prefer a hard error to catch bugs relating to that.

Let's not write difficult to debug code just to save a few keystrokes!

In cases where i'm using draw_sprite() to draw sprite_index, i'm almost always doing it to get some other expanded functionality that I can't get out of draw_self(), and so to me the same logic applies. I agree that not throwing an error when the sprite doesn't exist is probably going too far, but given that sprite_index is very commonly -1, I don't think it's good that your game will crash out with an error code just because you forgot to check whether it's -1 first.

Also yeah you caught me, I'm only recommending this in order to make it harder for people to find bugs 🙄. I don't appreciate the sarcasm, or characterizing a legitimate recommendation as "just wanting to save a few keystrokes".

backYard321 commented 7 months ago

A quick example to illustrate what i'm talking about:

You used to have to write:

if instance_exists(instance)
{
  with instance
  {}
}

or

if instance != noone
{
  with instance
    {}
}

You now no longer need to do this - with will simply not run the code if it's used with a negative number or a non-existent instance. You COULD say this makes bug detection harder, but it's implicit that if something doesn't exist or isn't associated with a resource/instance/struct, you would expect it not to run code and don't need it to throw an error, because the flexibility is worth it.

And, by extension, given that you just end up checking whether the sprite is -1 anyway, doesn't that counteract the benefit of having it error out in the first place?

gnysek commented 7 months ago

Isn't draw_self() just shortcut to draw_sprite_ext(sprite_index, image_index, x, y, image_xscale, image_yscale, image_angle, image_blend, image_alpha); ?

IMO it would be weird if former doesn't throw error, but latter does. However - I've no idea how to solve it better, as I understand that suggestion and how it makes harder to track down bugs.

katsaii commented 7 months ago

Please forgive the playful sarcasm. I don't doubt there is a valid use case for something like this, but I'm not sure supressing these errors is a good idea in the general case. Wouldn't you like to know if you accidentally pass in an invalid argument into one of these functions unintentionally?

If you really think that supressing the error would give you an ergonomic benefit, you could always create your own functions for drawing sprites:

function draw_sprite_or_nothing(sprite, subimg, x_, y_, xscale = 1, yscale = 1, rot = 0, colour = undefined, alpha = undefined) {
  colour ??= draw_get_colour();
  alpha ??= draw_get_alpha();
  return draw_sprite_ext(sprite, subimg, x_, y_, xscale, yscale, rot, colour, alpha);
}
backYard321 commented 7 months ago

Once again, my point isn't that I don't like typing out code - it's that I think games crashing whenever a sprite drawing function tries using -1 creates more problems than it solves, and by contrast I think it's intuitive that a sprite value of -1 would simply not draw (as @gynsek and I mentioned, there are other examples of Gamemaker being adjusted with this idea in mind).

Also, no offense, but I already know that I can write my own scripts as a workaround. My problem is that this is a common issue across many games I've played over the years, which I think could be improved (hence my feature request).

AtlaStar commented 7 months ago

Hard disagree; it makes perfect sense that it errors if the sprite_index is -1.

What wouldn't make sense is if sprite_index were to actually be storing some ref type with a null value and it errored. Basically, what I think is that sprites should become proper refs, and that unsetting the sprite_index should require using a proper function/constant that is a typed handle to a null sprite type. Then and only then would it be safe to not have it error if you pass in such a handle to things like draw_sprite

backYard321 commented 7 months ago

Functions like instance_place(), object_exists(), sprite_get_name(), and MANY others won't throw an error if they're passed a number not corresponding to a resource. And again, you now no longer have to check whether an instance or object exists in a lot of cases where you previously did. I don't think these changes are completely unacceptable for debugging - I think they've been significant improvements to Gamemaker, and I'd like to see sprite drawing functions get similarly improved.