int/object updating issue on XY axis

[Jordanowen42]

Hello all-

I'm working on a puzzle where the player is pushing a button to alter the position of a hatch that is blocking the flow of lava. The temperature and height of the lava within the pipe is determined by moving int counters. Each segment of the pipe has an odd number counter (the temp) and an even number counter (the height.) I've put all of this on an X/Y grid which I then transferred into a code, the first two segments of which are below. Right now I'm trying to have the code respond to a simple set of variables: "object 1" (the first hatch blocking flow from segment one to segment two) being visible should cause object 11, the segment, to show the graphic for 4/4 on the X/Y chart. That works. The problem is that I can't get the same response when object 2 is made visible.

Any help would be greatly appreciated.

Code: ags

function room_RepExec()
{
if (object[1].Visible == true) { myCounter1 = 4; myCounter2 = 4; return; }
if (object[2].Visible == true) { myCounter3 = 4; myCounter4 = 4; return; }

// Segment 1 integers
if (myCounter1 == 0 && myCounter2 == 0) {object[11].Graphic = 376; return;} 
else if (myCounter1 == 1 && myCounter2 == 1) {object[11].Graphic = 404; return;}
else if (myCounter1 == 1 && myCounter2 == 2) {object[11].Graphic = 398; return;}
else if (myCounter1 == 1 && myCounter2 == 3) {object[11].Graphic = 392; return;}
else if (myCounter1 == 1 && myCounter2 == 4) {object[11].Graphic = 386; return;}
else if (myCounter1 == 2 && myCounter2 == 1) {object[11].Graphic = 407; return;}
else if (myCounter1 == 2 && myCounter2 == 2) {object[11].Graphic = 401; return;}
else if (myCounter1 == 2 && myCounter2 == 3) {object[11].Graphic = 395; return;}
else if (myCounter1 == 2 && myCounter2 == 4) {object[11].Graphic = 389; return;}
else if (myCounter1 == 3 && myCounter2 == 1) {object[11].Graphic = 405; return;}
else if (myCounter1 == 3 && myCounter2 == 2) {object[11].Graphic = 399; return;}
else if (myCounter1 == 3 && myCounter2 == 3) {object[11].Graphic = 393; return;}
else if (myCounter1 == 3 && myCounter2 == 4) {object[11].Graphic = 387; return;}
else if (myCounter1 == 4 && myCounter2 == 1) {object[11].Graphic = 402; return;}
else if (myCounter1 == 4 && myCounter2 == 2) {object[11].Graphic = 396; return;}
else if (myCounter1 == 4 && myCounter2 == 3) {object[11].Graphic = 390; return;}
else if (myCounter1 == 4 && myCounter2 == 4) {object[11].Graphic = 384; return;}
else if (myCounter1 == 5 && myCounter2 == 1) {object[11].Graphic = 406; return;}
else if (myCounter1 == 5 && myCounter2 == 2) {object[11].Graphic = 400; return;}
else if (myCounter1 == 5 && myCounter2 == 3) {object[11].Graphic = 394; return;}
else if (myCounter1 == 5 && myCounter2 == 4) {object[11].Graphic = 388; return;}
else if (myCounter1 == 6 && myCounter2 == 1) {object[11].Graphic = 403; return;}
else if (myCounter1 == 6 && myCounter2 == 2) {object[11].Graphic = 397; return;}
else if (myCounter1 == 6 && myCounter2 == 3) {object[11].Graphic = 391; return;}
else if (myCounter1 == 6 && myCounter2 == 4) {object[11].Graphic = 385; return;}

// Segment 2 integers
if (myCounter1 == 0 && myCounter2 == 0) {object[12].Graphic = 476; return;}
else if (myCounter1 == 1 && myCounter2 == 1) {object[12].Graphic = 504; return;}
else if (myCounter1 == 1 && myCounter2 == 2) {object[12].Graphic = 498; return;}
else if (myCounter1 == 1 && myCounter2 == 3) {object[12].Graphic = 492; return;}
else if (myCounter1 == 1 && myCounter2 == 4) {object[12].Graphic = 486; return;}
else if (myCounter1 == 2 && myCounter2 == 1) {object[12].Graphic = 507; return;}
else if (myCounter1 == 2 && myCounter2 == 2) {object[12].Graphic = 501; return;}
else if (myCounter1 == 2 && myCounter2 == 3) {object[12].Graphic = 495; return;}
else if (myCounter1 == 2 && myCounter2 == 4) {object[12].Graphic = 489; return;}
else if (myCounter1 == 3 && myCounter2 == 1) {object[12].Graphic = 505; return;}
else if (myCounter1 == 3 && myCounter2 == 2) {object[12].Graphic = 499; return;}
else if (myCounter1 == 3 && myCounter2 == 3) {object[12].Graphic = 493; return;}
else if (myCounter1 == 3 && myCounter2 == 4) {object[12].Graphic = 487; return;}
else if (myCounter1 == 4 && myCounter2 == 1) {object[12].Graphic = 502; return;}
else if (myCounter1 == 4 && myCounter2 == 2) {object[12].Graphic = 496; return;}
else if (myCounter1 == 4 && myCounter2 == 3) {object[12].Graphic = 490; return;}
else if (myCounter1 == 4 && myCounter2 == 4) {object[12].Graphic = 484; return;}
else if (myCounter1 == 5 && myCounter2 == 1) {object[12].Graphic = 506; return;}
else if (myCounter1 == 5 && myCounter2 == 2) {object[12].Graphic = 500; return;}
else if (myCounter1 == 5 && myCounter2 == 3) {object[12].Graphic = 494; return;}
else if (myCounter1 == 5 && myCounter2 == 4) {object[12].Graphic = 488; return;}
else if (myCounter1 == 6 && myCounter2 == 1) {object[12].Graphic = 503; return;}
else if (myCounter1 == 6 && myCounter2 == 2) {object[12].Graphic = 497; return;}
else if (myCounter1 == 6 && myCounter2 == 3) {object[12].Graphic = 491; return;}
else if (myCounter1 == 6 && myCounter2 == 4) {object[12].Graphic = 485; return;}

If it helps, here's the code for the whole room:

Code: ags

// room script file

// Segment 1
int myCounter1 ;
int myCounter2 ;

// Segment 2
int myCounter3 ;
int myCounter4 ;

// Segment 3
int myCounter5 ;
int myCounter6 ;

// Segment 4
int myCounter7 ;
int myCounter8 ;

// Segment 5
int myCounter9 ;
int myCounter10 ;

// Segment 6
int myCounter11 ;
int myCounter12 ;

// Segment 7
int myCounter13 ;
int myCounter14 ;

// Segment 8
int myCounter15 ;
int myCounter16 ;

// Segment 9
int myCounter17 ;
int myCounter18 ;



function oObject20_AnyClick()
{
if (object[20].Graphic == 410)
{object[20].Graphic = 411;
object[1].Visible = false;
object[2].Visible = true;
object[3].Visible = false;
return;}

if (object[20].Graphic == 411)
{object[20].Graphic = 412;
object[2].Visible = false;
object[3].Visible = true;
return;}

if (object[20].Graphic == 412)
{object[20].Graphic = 410;
object[1].Visible = true;
object[3].Visible = false;
return;}



}

function oObject21_AnyClick()
{
if (object[21].Graphic == 410)
{object[21].Graphic = 411;
object[4].Visible = false;
object[5].Visible = true;
object[6].Visible = false;
return;}

if (object[21].Graphic == 411)
{object[21].Graphic = 412;
object[4].Visible = false;
object[5].Visible = false;
object[6].Visible = true;
return;}

if (object[21].Graphic == 412)
{object[21].Graphic = 410;
object[4].Visible = true;
object[5].Visible = false;
object[6].Visible = false;
return;}

}

function oObject22_AnyClick()
{
if (object[22].Graphic == 410)
{object[22].Graphic = 411;
object[7].Visible = false;
object[8].Visible = true;
object[9].Visible = false;
return;}

if (object[22].Graphic == 411)
{object[22].Graphic = 412;
object[7].Visible = false;
object[8].Visible = false;
object[9].Visible = true;
return;}

if (object[22].Graphic == 412)
{object[22].Graphic = 410;
object[7].Visible = true;
object[8].Visible = false;
object[9].Visible = false;
return;}

}

function room_Load()
{


 
}

function room_RepExec()
{
if (object[1].Visible == true) { myCounter1 = 4; myCounter2 = 4; return; }
if (object[2].Visible == true) { myCounter3 = 4; myCounter4 = 4; return; }

// Segment 1 integers
if (myCounter1 == 0 && myCounter2 == 0) {object[11].Graphic = 376; return;} 
else if (myCounter1 == 1 && myCounter2 == 1) {object[11].Graphic = 404; return;}
else if (myCounter1 == 1 && myCounter2 == 2) {object[11].Graphic = 398; return;}
else if (myCounter1 == 1 && myCounter2 == 3) {object[11].Graphic = 392; return;}
else if (myCounter1 == 1 && myCounter2 == 4) {object[11].Graphic = 386; return;}
else if (myCounter1 == 2 && myCounter2 == 1) {object[11].Graphic = 407; return;}
else if (myCounter1 == 2 && myCounter2 == 2) {object[11].Graphic = 401; return;}
else if (myCounter1 == 2 && myCounter2 == 3) {object[11].Graphic = 395; return;}
else if (myCounter1 == 2 && myCounter2 == 4) {object[11].Graphic = 389; return;}
else if (myCounter1 == 3 && myCounter2 == 1) {object[11].Graphic = 405; return;}
else if (myCounter1 == 3 && myCounter2 == 2) {object[11].Graphic = 399; return;}
else if (myCounter1 == 3 && myCounter2 == 3) {object[11].Graphic = 393; return;}
else if (myCounter1 == 3 && myCounter2 == 4) {object[11].Graphic = 387; return;}
else if (myCounter1 == 4 && myCounter2 == 1) {object[11].Graphic = 402; return;}
else if (myCounter1 == 4 && myCounter2 == 2) {object[11].Graphic = 396; return;}
else if (myCounter1 == 4 && myCounter2 == 3) {object[11].Graphic = 390; return;}
else if (myCounter1 == 4 && myCounter2 == 4) {object[11].Graphic = 384; return;}
else if (myCounter1 == 5 && myCounter2 == 1) {object[11].Graphic = 406; return;}
else if (myCounter1 == 5 && myCounter2 == 2) {object[11].Graphic = 400; return;}
else if (myCounter1 == 5 && myCounter2 == 3) {object[11].Graphic = 394; return;}
else if (myCounter1 == 5 && myCounter2 == 4) {object[11].Graphic = 388; return;}
else if (myCounter1 == 6 && myCounter2 == 1) {object[11].Graphic = 403; return;}
else if (myCounter1 == 6 && myCounter2 == 2) {object[11].Graphic = 397; return;}
else if (myCounter1 == 6 && myCounter2 == 3) {object[11].Graphic = 391; return;}
else if (myCounter1 == 6 && myCounter2 == 4) {object[11].Graphic = 385; return;}

// Segment 2 integers
if (myCounter1 == 0 && myCounter2 == 0) {object[12].Graphic = 476; return;}
else if (myCounter1 == 1 && myCounter2 == 1) {object[12].Graphic = 504; return;}
else if (myCounter1 == 1 && myCounter2 == 2) {object[12].Graphic = 498; return;}
else if (myCounter1 == 1 && myCounter2 == 3) {object[12].Graphic = 492; return;}
else if (myCounter1 == 1 && myCounter2 == 4) {object[12].Graphic = 486; return;}
else if (myCounter1 == 2 && myCounter2 == 1) {object[12].Graphic = 507; return;}
else if (myCounter1 == 2 && myCounter2 == 2) {object[12].Graphic = 501; return;}
else if (myCounter1 == 2 && myCounter2 == 3) {object[12].Graphic = 495; return;}
else if (myCounter1 == 2 && myCounter2 == 4) {object[12].Graphic = 489; return;}
else if (myCounter1 == 3 && myCounter2 == 1) {object[12].Graphic = 505; return;}
else if (myCounter1 == 3 && myCounter2 == 2) {object[12].Graphic = 499; return;}
else if (myCounter1 == 3 && myCounter2 == 3) {object[12].Graphic = 493; return;}
else if (myCounter1 == 3 && myCounter2 == 4) {object[12].Graphic = 487; return;}
else if (myCounter1 == 4 && myCounter2 == 1) {object[12].Graphic = 502; return;}
else if (myCounter1 == 4 && myCounter2 == 2) {object[12].Graphic = 496; return;}
else if (myCounter1 == 4 && myCounter2 == 3) {object[12].Graphic = 490; return;}
else if (myCounter1 == 4 && myCounter2 == 4) {object[12].Graphic = 484; return;}
else if (myCounter1 == 5 && myCounter2 == 1) {object[12].Graphic = 506; return;}
else if (myCounter1 == 5 && myCounter2 == 2) {object[12].Graphic = 500; return;}
else if (myCounter1 == 5 && myCounter2 == 3) {object[12].Graphic = 494; return;}
else if (myCounter1 == 5 && myCounter2 == 4) {object[12].Graphic = 488; return;}
else if (myCounter1 == 6 && myCounter2 == 1) {object[12].Graphic = 503; return;}
else if (myCounter1 == 6 && myCounter2 == 2) {object[12].Graphic = 497; return;}
else if (myCounter1 == 6 && myCounter2 == 3) {object[12].Graphic = 491; return;}
else if (myCounter1 == 6 && myCounter2 == 4) {object[12].Graphic = 485; return;}




}

[pell]

I can see that on Line 128 of the full room code listing that object[2] relates to the variables myCounter3 and myCounter4. But I don't see where you handle either of those variables anywhere else in the code you posted (besides declaring them).

[heltenjon]

Edit: deleted, I was wrong.

[pell]

Edit: deleted, I was wrong.

In what way?

Edit: Never mind. You caught yourself as I was posting.

[Khris]

Yes, you're still checking myCounter1 and myCounter2, not myCounter3 and myCounter4.

---

Sorry, but please do yourself a favor and learn how to not write code like that.
Leaving aside how tedious this is to maintain, it's also really prone to typos.

First of all, please do not use variables like "myCounter1", "myCounter2", etc. It's a temperature and a height, so why not actually use "temp1" and "height1" for starters? Or t1 and h1?

Also, if you organize your sprite numbers, you can replace this *entire block* with a single line like

Code: ags

  object[11].Graphic = 384 + t1 * 4 + h1;

You're already using an X/Y chart, now do the same for your sprite numbers. Instead of using 1 as the lowest number, make everything zero-based, at least internally.
The temperature has six different values, so 0-5. The height has four different values, 0-3.
Multiplying the temperature by 4 (the amount of height values) and adding the height will produce the numbers 0 to 23. Now simply add the first sprite number as offset and there your go.
If you add all the sprites to AGS in the correct order you won't even have to edit the sprite numbers.

TL;DR: Debugging code like this is a pointless exercise.

---

Also note that this code does not have to be inside repeatedly_execute/room_RepExec. The object's .Graphic only changes after one of the ints has changed, so that is the only point in time where you actually need to update it: right after an int change.

[Snarky]

pell has found at least one issue, but I would point out a couple of other problems with this code.

First, please fix your indentation. Stuff like this is awful:

Code: ags

function oObject20_AnyClick()
{
if (object[20].Graphic == 410)
{object[20].Graphic = 411;

Second, you're using a lot of return statements all over the place. An old programming principle, due to Dijkstra, is that every function should have only one return statement. In practice it is often convenient to bend this rule, but it is still something to keep in mind. Having too many places a function can exit can make the program flow hard to follow. And most of these return statements are unnecessary because they're part of an if/else-if structure: only the block under the first condition that evaluates as true will run anyway.

And finally, you're using room_RepExec() to set the state. An old AGS rule of thumb, due to me, is that if you're using one of the repeatedly executing functions, you're probably doing something "wrong" (meaning: not in a good way). Here, the state only changes when the player clicks on something, so you should put all the common update code into a function and call that whenever an interaction can change the state.

In this case, those two rules combine in a particularly serious way: you're putting multiple return statements into a repeatedly executing function. So as soon as it hits a return statement, it will stop running the rest of the function, and this will keep happening every game cycle. (One reason this is bad is that the repeatedly executing functions very often contain logic for totally unrelated things that need to update, so you don't want to accidentally disable them by hitting a return.)

So in this case, if (object[1].Visible == true) or (object[2].Visible == true), none of the rest of the function will run, since you have a return at the end of each of those blocks. In fact, I don't see how this could work as you describe at all:

"object 1" (the first hatch blocking flow from segment one to segment two) being visible should cause object 11, the segment, to show the graphic for 4/4 on the X/Y chart. That works.

It should set the myCounter1 and myCounter2 values but not do anything else. Only when you set it (and object[2]) to not be visible should the graphics update.

[Khris]

Right, the "return" kills the second part, I didn't even catch that

[pell]

So in this case, if (object[1].Visible == true) or (object[2].Visible == true), none of the rest of the function will run, since you have a return at the end of each of those blocks. In fact, I don't see how this could work as you describe at all:

Right, the "return" kills the second part, I didn't even catch that

To be fair, at least as far as  I understand the code, that looks like the intended behavior. The visibility of those two objects are set outside this function based on user clicks. If they are false, this function changes the sprites based on certain conditions, and if either are true, it returns without changing any of the sprites.

[Snarky]

Oh, there was one more thing I forgot to mention: this problem would be pretty easy to debug yourself by setting a breakpoint and stepping through the code, except that it's a pain to do that in a repeatedly executing function because it keeps pausing the game constantly before you get to the point you want to test. This is another good reason to avoid using room_RepExec() unless absolutely necessary.

[Khris]

To be fair, at least as far as  I understand the code, that looks like the intended behavior. The visibility of those two objects are set outside this function based on user clicks. If they are false, this function changes the sprites based on certain conditions, and if either are true, it returns without changing any of the sprites.

As Snarky already pointed out, the code will call "return;" in a ton of cases and thus exit the function before other variables are handled. This is almost guaranteed to not be intentional and very likely the reason why OP is posting in the first place; OP is just copy-pasting code without fully understanding what it does.

[pell]

To be fair, at least as far as  I understand the code, that looks like the intended behavior. The visibility of those two objects are set outside this function based on user clicks. If they are false, this function changes the sprites based on certain conditions, and if either are true, it returns without changing any of the sprites.

As Snarky already pointed out, the code will call "return;" in a ton of cases and thus exit the function before other variables are handled. This is almost guaranteed to not be intentional and very likely the reason why OP is posting in the first place; OP is just copy-pasting code without fully understanding what it does.

The OP claimed the code worked fine for the first object, but not for the second. I'm thinking the bulk of the body of the function probably needs to be repeated but using myCounter3 and myCounter4.

[Khris]

Yes, but again: the code for the first object is very likely to also call "return;" which will exit the function and thus ensure that the code for the second object is never executed in the first place. Doesn't matter if the variable names are fixed if the code never runs.

[pell]

Yes, but again: the code for the first object is very likely to also call "return;" which will exit the function and thus ensure that the code for the second object is never executed in the first place. Doesn't matter if the variable names are fixed if the code never runs.

Are you seeing a short circuit that I'm missing? Most of the return statements are superfluous. After the first two statements that check for visibility, all the other returns could be deleted (and should be) with no change to control flow that I can see (edit: only because the function just returns anyway after all these if/else statements).

Edit: I think the behavior in your first sentence is what the OP intends from what I can see.

Edit again: The OP can clarify, but it looks to me like it's:
 If object 1 visible, do nothing.
  If object 2 visible, do nothing.
 Otherwise, do all this stuff. (Except there is probably missing stuff causing the missing behavior.)

[Khris]

@pell
My point is that in the "all this stuff" part, if any of the lines concerning segment #1 are a match, the code for segment #2 never runs. This is also most likely what caused OP to open this thread.
Because why would they deliberately sabotage their segment2 code, then ask here why it doesn't work?
None of this is intentional, because it's all a hot, flaming mess. Again, sorry. But this needs to be unlearned ASAP.

As an aside: using the object[] array in a room script is possible but less readable than using oButton1 or oSegment2 or whatever.

[Snarky]

@Khris, the code will run if object[1] is turned off again, so @pell is right that it could in theory be intended behavior. (I have written similar logic for things like mouse gestures that should only trigger once the button is released.) It's not how I read the description, but it might be glossing over that detail, and if Jordanowen says it does work...

I also took this bit:

Right now I'm trying to have the code respond to a simple set of variables

... to mean that this is only the first step of building the complete logic, so that the lines are temporary/incomplete in order to test that the idea works.

Of course you are right that it should all be scrapped and replaced by a simple expression. An alternative to doing sprite ID arithmetic (potentially requiring a reimport of all the sprites to get them in the right order) would be to load the sprites into a view, and map e.g. the temperature to the loop number and the height to the frame number. (So for temperature 2 and height 3 you would use loop 2 frame 3.) That way you would be able to see the sprite variations in a table-like layout in the editor, and use the animation preview to inspect them.

[pell]

@pell
My point is that in the "all this stuff" part, if any of the lines concerning segment #1 are a match, the code for segment #2 never runs. This is also most likely what caused OP to open this thread.
Because why would they deliberately sabotage their segment2 code, then ask here why it doesn't work?
None of this is intentional, because it's all a hot, flaming mess. Again, sorry. But this needs to be unlearned ASAP.

As an aside: using the object[] array in a room script is possible but less readable than using oButton1 or oSegment2 or whatever.

I think we're talking past each other. My point is that the code for object 2 is probably not running because it's probably missing. All the "stuff"  looks to me like it applies to object1.

I do need to clarify that when I said the control flow is the same with or without most of those return statments, I meant that the function just returns after the if/else statements anyway. Of course, if any other statements are added afterwards, those returns within the bodies of the if statements would shortcircuit the control.

[Khris]

OP was copy-pasting the two dozen lines then replacing 11 with 12 but forgot to also rename the variables is my assessment.
So the second block is very much supposed to run in all cases and supposed to handle the second lava pipe segment (object[12]) but doesn't because the segment #1 / object[11] code always prematurely exits the function.

If I'm wrong it doesn't really matter anyway because this entire exchange is moot. This approach is fundamentally flawed and using it should not be encouraged even if certain minor aspects seem like a good idea. I hate to belabor this point but writing code like this needs not to happen, ever.

As an aside, I do not adhere to the "only one return statement" principle because it is bound to create a bunch of nesting but at the same time one obviously shouldn't use returns indiscriminately.


Using a view to connect the two variables and the according sprite is a great idea!

[pell]

OP was copy-pasting the two dozen lines then replacing 11 with 12 but forgot to also rename the variables is my assessment.
So the second block is very much supposed to run in all cases and supposed to handle the second lava pipe segment (object[12]) but doesn't because the segment #1 / object[11] code always prematurely exits the function.

This caused me to look back again. I missed that there was a second part in scrolling because it looks like the first part since the OP forgot (presumably) to change the variable names. That's the "missing part".

The solution is surely to change those variable names to myCounter3 and myCounter4 unless we've overlooked something else.

[Crimson Wizard]

In regards to the problem of returning from the middle of "repeatedly execute":
If there's multiple unrelated stuff updated there, like for example: update puzzle, update character position, update blinking lights on background, and so forth, then I do recommend moving unrelated parts of code into separate functions and call these functions from rep-exec. Then each part will be updated separately, not conflicting with each other.

For a dumb example:

Code: ags

function UpdateTwoObjects()
{
    // Idk if this code itself is correct, I'm just copy/pasting it for this example
    if (object[1].Visible == true) { myCounter1 = 4; myCounter2 = 4; return; }
    if (object[2].Visible == true) { myCounter3 = 4; myCounter4 = 4; return; }
}

function UpdateCounters()
{
    // all the counter stuff
}

function room_RepExec()
{
    UpdateTwoObjects();
    UpdateCounters();
}

If updating counters should be skipped awhole under some condition (like objects visible or not), then you may solve that by adding a early return like this:

Code: ags

function UpdateCounters()
{
    // don't update counters if something is off
    if (some condition) { return; }

    // all the counter stuff
}

PS. By the way, this "early return" code style is one that opposes the aforementioned Djikstra's proposal, apparently.
This pattern has its name in computer science, but I keep forgetting it.

[Jordanowen42]

@pell
My point is that in the "all this stuff" part, if any of the lines concerning segment #1 are a match, the code for segment #2 never runs. This is also most likely what caused OP to open this thread.
Because why would they deliberately sabotage their segment2 code, then ask here why it doesn't work?
None of this is intentional, because it's all a hot, flaming mess. Again, sorry. But this needs to be unlearned ASAP.

As an aside: using the object[] array in a room script is possible but less readable than using oButton1 or oSegment2 or whatever.

I changed the segment 2 script so it's myCounter 3 and 4. Still nothing. Going to implement some more ideas.