A script crashes the engine

[CrackedP0t]

When I run this code, the game crashes.

Code: Select all

int badshots = ActorCount("badshot");
int i;
char*clonename;
Actor*clone;
Actor*closestshot;
int smalldist;
for (i = 0; i < badshots; i++) {
    sprintf(clonename, "badshot.%d", i);
    clone = getclone(clonename);
    if (distance(x,y,clone->x,clone->y) < smalldist) {
        closestshot = clone;
        smalldist = distance(x,y,clone->x,clone->y);
                                }
                               }


Explanation:
There are actors called "badshot" flying around the screen--they're created every so often. The code above is supposed to figure out which "badshot" is closest to it. They and it are constantly moving, so the code is put in the Draw Actor event. If the code above is hopeless, what's another way do figure out which "badshot" is closest?

[DarkParadox]

Well, your crash bug is something I run into a lot myself. You can't use sprintf on a char*, only char arrays. There was a couple other problems too, which I fixed.

• clonename is already used as an actor variable, it didn't alert you because code bugs anyways. I changed it to clone_name.
• If you didn't know, variables can be declared on the same line seperated by commas. E.g. if you wanted to declare two int variables, you can used int v1, v2;
• You didn't set a value to smalldist, this means that it will be set to a totally random number (called a garbage value), so when you end up comparing it it could, for example, a negative value. Make sure to check if you should initialize a value when you write code, or it could break easily. In this instance, you should set smalldist to the largest distance possible for badshot's to be away; I just set it to 99999.
• Another bug that comes up is that you looped through clones incorrectly. When you loop through clones and access your values, it'll try to get every clone up to the number of actors. So what happens when one of the clones doesn't exist anymore and you try to access its variable? The code breaks. You can prevent this by checking every clone against "NULL", to see if it exists.
• You'll also notice I removed the badshots = ActorCount() part of your code. You need to declare a new variable in the GUI called badshots, and on the create actor event of the badshot actor, do badshots += 1. To be safe, you'll also probably want to do badshots=0 on your view's create actor event, to make sure we don't get a garbage value like I pointed out earlier with smalldist. Why do we need this? Because the ActorCount of badshot isn't always the highest cloneindex for the badshot's. If a clone is destroyed, that clone index no longer exists. Clones are always added to the end.
  OR you could go into global code and add a script that says int badshots = 0; and add it. You can use global code to get around the GUI and set variables right away.

Fixed code, follow directions in the last part of the list:

Code: Select all

int i;
char clone_name[64];
Actor* clone;
Actor* closestshot;
int smalldist = 99999;
for (i = 0; i < badshots; i++) {
  sprintf(clone_name, "badshot.%d", i);
  clone = getclone(clone_name);
  if (clone != NULL) {
    if (distance(x,y,clone->x,clone->y) < smalldist) {
      closestshot = clone;
      smalldist = distance(x,y,clone->x,clone->y);
    }
  }
}


P.S.
I always thought it was odd how passing a char* to sprintf would crash everything.

[lcl]

P.S.
I always thought it was odd how passing a char* to sprintf would crash everything.

That's because char* is a pointer. You have to allocate memory for pointers to point to before you can write anything to them. GE should warn you of that but I guess it's just something that has been forgotten when the error checkings have been written.

You could do this:

Code: Select all

clone_name = malloc(sizeof(char) * 200);

But that would be just the same as declaring the variable like this:

Code: Select all

char clone_name[200];

And when allocating memory, one must also remember to free it, or otherwise a memory leak would be created. So, it's more logical, and easier to just use an array in cases like that.

[CrackedP0t]

This works great, but there's one problem: There's a random point at ~(0,300) that is will pick as a "badshot", despite the fact that there's no shot there.

[DarkParadox]

P.S.
I always thought it was odd how passing a char* to sprintf would crash everything.

That's because char* is a pointer. You have to allocate memory for pointers to point to before you can write anything to them. GE should warn you of that but I guess it's just something that has been forgotten when the error checkings have been written.

Ah, I didn't realize sprintf didn't allocate memory for pointers automatically like strcpy. I should have figured that out a while ago.
I know how to use malloc for dynamic arrays and what-not, though.

[skydereign]

Ah, I didn't realize sprintf didn't allocate memory for pointers automatically like strcpy

strcpy does not allocate memory for your pointers. It will work in some weird individual cases, but you should never rely on it. For instance this program segfaults.

Code: Select all

int
main ()
{
    char* test1;
    char* test2;
    strcpy(test1, "test1");
    strcpy(test2, "test2");
    return 0;
}

And in gameEditor when you try to strcpy into a pointer that is not assigned to valid memory, it will copy nothing. Therefore this will crash because sprintf will try writing a null string into text.

Code: Select all

char* test;
strcpy(test, "test text");
sprintf(text, "test = [%s]", test);