Matching Masks Puzzle

[Ghostlady]

I created this puzzle where there are 15 different masks (like a masquerade ball mask) and you have to click on two identical masks to make a pair.  These objects/masks will change their graphic based on a timer, some going faster than others.  So the masks are constantly flipping their graphic on the screen and you have to click on them in time.  It seems sometimes it works and and sometimes it doesn't.  Maybe the click happens during the graphic change or my script is wrong.  I was wondering if someone can look at it and see what I am doing wrong.  

I can't post the script here because it exceeded the allotted amount of characters.

[Khris]

I can't imagine why it would be that long :o

Anyway, you can paste it here: http://codetidy.com/
Then we can take a look at it.

[monkey0506]

Well we would really need to be able to see the code one way or another to be able to help you. You can Google "online clipboard" and find several and various sites that will allow you to paste your code and then give us a link to view it.

To be entirely honest, it sounds like you could benefit a lot from using some generics instead of programming things entirely statically, but that may not be the case. Again, we'd need to be able to see the code to tell.

Edit: Khris beat me, but yes.

[Ghostlady]

Here is the Link:

http://codetidy.com/897/

The script will keep track whether it is a first click or the second click.  The first click sets the graphic that it has to match.  If it finds its match, then click 1 and click2 will clear out.  If it doesn't match, then click1 should be the starting graphic again.

[Ghostlady]

Here are two screenshots (in game play):

[monkey0506]

A first thought...

Code: ags

function Mask0a()
{
      if (oMask0.Graphic == 523) {
          graphic1 = 523; }
 
      if (oMask0.Graphic == 524) {
          graphic1 = 524; }
 
      if (oMask0.Graphic == 525) {
          graphic1 = 525; }
 
      if (oMask0.Graphic == 526) {
          graphic1 = 526; }
 
      if (oMask0.Graphic == 527) {
          graphic1 = 527; }
 
      if (oMask0.Graphic == 528) {
          graphic1 = 528; }
          
      if (oMask0.Graphic == 529) {
          graphic1 = 529; }
          
      if (oMask0.Graphic == 530) {
          graphic1 = 530; }
 
      if (oMask0.Graphic == 531) {
          graphic1 = 531; }
 
      if (oMask0.Graphic == 532) {
          graphic1 = 532; }
 
      if (oMask0.Graphic == 533) {
          graphic1 = 533; }
 
      if (oMask0.Graphic == 534) {
          graphic1 = 534; }
 
      if (oMask0.Graphic == 535) {
          graphic1 = 535; }
 
      if (oMask0.Graphic == 536) {
          graphic1 = 536; }
 
      if (oMask0.Graphic == 537) {
          graphic1 = 537; }
}

This entire function can be simplified to:

Code: ags

function Mask0a()
{
  graphic1 = oMask0.Graphic;
}

That will save you, oh I don't know, probably about a thousand lines of code.

[monkey0506]

I haven't actually compiled or tested it, but this should be pretty much the same as the existing code, just cleaned up:

Code: ags

// room script file
int counter;
int click1;
int graphic1;
int graphic2;
Object *clickedMasks[2];

function Clear()
{
  click1 = 0;
  clickedMasks[0] = null;
  clickedMasks[1] = null;
}

int GetTimerLimit(int timer)
{
  if (timer == 1) return 60;
  if (timer == 2) return 80;
  if (timer == 3) return 90;
  if (timer == 4) return 65;
  if (timer == 5) return 100;
  if (timer == 6) return 60;
  if (timer == 7) return 100;
  if (timer == 8) return 90;
  if (timer == 9) return 75;
  if (timer == 10) return 100;
  if (timer == 11) return 85;
  if (timer == 12) return 70;
  if (timer == 13) return 90;
  if (timer == 14) return 80;
  if (timer == 15) return 60;
}

function room_Load()
{
  aDanse_Espagnole.Play();
  gScavOnGui.Visible = false;
  int i = 1;
  while (i <= 15)
  {
    SetTimer(i, GetTimerLimit(i));
    i++;
  }
}

void ShowNext(this Object*)
{
  this.Graphic++;
  if (this.Graphic == 538) this.Graphic = 523;
}

Object* GetObjectByMaskID(int maskID)
{
  if (maskID == 0) return oMask0;
  if (maskID == 1) return oMask1;
  if (maskID == 2) return oMask2;
  if (maskID == 3) return oMask3;
  if (maskID == 4) return oMask4;
  if (maskID == 5) return oMask5;
  if (maskID == 6) return oMask6;
  if (maskID == 7) return oMask7;
  if (maskID == 8) return oMask8;
  if (maskID == 9) return oMask9;
  if (maskID == 10) return oMask10;
  if (maskID == 11) return oMask11;
  if (maskID == 12) return oMask12;
  if (maskID == 13) return oMask13;
  if (maskID == 14) return oMask14;
}

void CheckTimersAndUpdateMasks()
{
  int i = 1;
  while (i <= 15)
  {
    if (IsTimerExpired(i))
    {
      SetTimer(i, GetTimerLimit(i));
      Object *o = GetObjectByMaskID(i - 1);
      o.ShowNext();
    }
    i++;
  }
}

void SetMasksVisibility(bool visible)
{
  int i = 0;
  while (i < 15)
  {
    Object *o = GetObjectByMaskID(i);
    o.Visible = visible;
    i++;
  }
}

function room_RepExec()
{
  if ((GUI.GetAtScreenXY(mouse.x, mouse.y) != null) && (mouse.Mode != eModeUseinv)) mouse.UseModeGraphic(eModePointer);
  if (mouse.Mode == eModeUseinv) mouse.UseDefaultGraphic();
  if (IsKeyPressed(eKeyTab)) counter = 32;
  if (IsKeyPressed(eKeyHome)) player.ChangeRoom(7);
  CheckTimersAndUpdateMasks();
  if ((click == 2) && (graphic1 == graphic2))
  {
    aSound97.Play();
    counter++;
    click1 = 0;
    graphic1 = 998;
    graphic2 = 999;
    clickedMasks[0].ShowNext();
    clickedMasks[1].ShowNext();
  }
  if (counter == 32)
  {
    SetMasksVisibility(false);
    Wait(80);
    int i = 0;
    while (i < 15)
    {
      Object *o = GetObjectByMaskID(i);
      o.Graphic = (523 + i);
      i++;
    }
    SetMasksVisibility(true);
    Wait(120);
    SetMasksVisibility(false);
    Wait(80);
    aDanse_Espagnole.Stop();
    gVase.Visible = true;
    aSound65.Play();
    MaskDone = 1;
    gVase.BackgroundGraphic = 294;
    Wait(100);
    gVase.Visible = false;
    Wait(30);
    BtnMask.NormalGraphic = 298;
    gScavenger.Visible = true;
    Wait(160);
    gScavenger.Visible = false;
    player.AddInventory(iMask);
    InvSwitch = 1;
    player.ChangeRoom(249);
  }
  if (click1 == 2) Clear();
}

void Mask_Interact(Object *mask)
{
  if ((click1 == 1) && (clickedMasks[0] != mask))
  {
    graphic2 = mask.Graphic;
    clickedMask[1] = mask;
    click1 = 2;
  }
  else
  {
    graphic1 = mask.Graphic;
    clickedMask[0] = mask;
    click1 = 1;
  }
}

function oMask0_Interact()
{
  Mask_Interact(oMask0);
}

function oMask1_Interact()
{
  Mask_Interact(oMask1);
}

function oMask2_Interact()
{
  Mask_Interact(oMask2);
}

function oMask3_Interact()
{
  Mask_Interact(oMask3);
}

function oMask4_Interact()
{
  Mask_Interact(oMask4);
}

function oMask5_Interact()
{
  Mask_Interact(oMask5);
}

function oMask6_Interact()
{
  Mask_Interact(oMask6);
}

function oMask7_Interact()
{
  Mask_Interact(oMask7);
}

function oMask8_Interact()
{
  Mask_Interact(oMask8);
}

function oMask9_Interact()
{
  Mask_Interact(oMask9);
}

function oMask10_Interact()
{
  Mask_Interact(oMask10);
}

function oMask11_Interact()
{
  Mask_Interact(oMask11);
}

function oMask12_Interact()
{
  Mask_Interact(oMask12);
}

function oMask13_Interact()
{
  Mask_Interact(oMask13);
}

function oObject14_Interact()
{
  Mask_Interact(oMask14);
}

I'd actually suggest merging the Object_Interact functions into a single handler (copy and paste the name of the single handler into the Events pane for each object), and then merge that function with the Mask_Interact function that I've already written. Something like:

Code: ags

void Mask_Interact()
{ // link this event in the object event handlers
  Object *mask = Object.GetAtScreenXY(mouse.x, mouse.y);
  if ((click1 == 1) && (clickedMasks[0] != mask))
  {
    graphic2 = mask.Graphic;
    clickedMask[1] = mask;
    click1 = 2;
  }
  else
  {
    graphic1 = mask.Graphic;
    clickedMask[0] = mask;
    click1 = 1;
  }
}

The reason I left the existing handlers is because I wanted to leave it in a consistent state with the way you already had your project established.

I tried to minimize any changes in the code other than just generalizing it and cleaning it up, but let me know if I've broken something.

Edit: By the way..just a fun thought. Your original source was 2801 lines, my version of the code is 236 lines (including the multiple event handlers), a reduction of 91.5%. Sometimes writing things a bit more generically can be useful.

[Ghostlady]

I can't get to test this tonight but I will by tomorrow.  I am sure I'll have some questions.

The code looks a lot cleaner and I can slap myself in the forehead because I should have gotten "graphic1 = oMask0.Graphic;"

[monkey0506]

The bit towards the end where checking if (counter == 32) may not work the same because you were confusing Object.Visible and Object.Graphic (a lot ), so I kind of had to guess between what was actually happening and what you meant.

Also, the GetObjectByMaskID function isn't necessary if your mask objects are all numbered sequentially (i.e., oMask0 is object 0, oMask1 is object 1, ..., oMask14 is object 14) because then you could simply use the object array:

Code: ags

// CheckTimersAndUpdateMasks
    if (IsTimerExpired(i))
    {
      SetTimer(i, GetTimerLimit(i));
      object[i - 1].ShowNext();
    }

If they are numbered sequentially but oMask0 isn't object 0, and is, say object 10, then you could simply offset the index by +10:

Code: ags

// CheckTimersAndUpdateMasks
    if (IsTimerExpired(i))
    {
      SetTimer(i, GetTimerLimit(i));
      object[i + 9].ShowNext(); // i + 10 - 1 == i + 9
    }

There's a couple of other areas in the script that I would have done things a bit differently as well, but again one of my main goals was keeping things consistent with the way your project was established in the editor. Without being able to see some of it, I had to take other measures for some things (like the GetObjectByMaskID function).

[Peter Bear]

For your fear of the "click during Mask switching", you could manage this function to be blocking ... so the player won't be able to click or do anything during the switch ( which will be very fast anyway )

see http://www.adventuregamestudio.co.uk/manual/BlockingScripts.htm
Maybe I could be of some help.

[monkey0506]

Given that every single mask is being switched at a different time interval, there's nothing that could really be done to make the switch into a blocking type of event. That quite simply doesn't make any sense. How would you propose going about making:

Code: ags

oMask0.Graphic++;

Into a blocking event? What would you be blocking? It would show the current graphic until the screen was redrawn on the next game loop at which time the new graphic would be shown. The switch itself is for all intents and purposes an instantaneous event. How do you block a singular instant that lasts less than a single game loop?

If the user clicks on a mask at the moment in which the graphic is changed there are only and exactly three possible outcomes:

1) the user clicks in the same loop that the graphic will be changed, but the mouse-click is processed first. The clicked graphic ID will be set as the user would have expected.

2) the user clicks in the same loop that the graphic has already been changed, and the mouse-click has been processed after the graphic was changed. The clicked graphic ID will be set to the newly displayed graphic, but the user should almost immediately (within 1/40th of a second) become aware that the graphic has just changed, and should be able to realize that the click may have been too late.

3) the user clicks in the loop after the graphic has been changed. By the time the user actually clicked the mouse, the new graphic has already been drawn to the screen. As with (2), the user should become aware that the graphic has changed, and their click may have been wasted.

I'm reasonably certain that repeatedly_execute is processed before on_mouse_click, but that may not be the case. If it is, then (1) would never even actually be a viable possibility, and there would only be the latter two possible outcomes. Regardless of any of the three though, the user should still be able to recognize (again, within 1/40th of a second) that the click may have been wasted. This isn't a flaw in your game design or some type of bug. That's something that you can reasonably expect of someone playing your game to understand.

[Ghostlady]

Getting an "undefined token" error on 'SetMasksVisibility' when it is doing this part of the code:

SetMasksVisiblity(true);

[arj0n]

Getting an "undefined token" error on 'SetMasksVisibility' when it is doing this part of the code:

SetMasksVisiblity(true);

SetMasksVisiblity(true); =
SetMasksVisibility(true);

[Ghostlady]

Not sure what you mean Arj0n.  I commented out that area for now since it doesn't get executed until the puzzle is completed.

So the puzzle seems to be reacting the same way as before.  Sometimes it recognizes the match and sometimes it doesn't.  If you get all good matches it works, but if you mess up and click a wrong match then it seems to get confused.

Plus, I get this error:

Error: Error running function  'room_RepExec'
Error: Script appears to be hung ( a while loop 150001 times).  The problem may be in calling a function; check the call stack.

[monkey0506]

He was trying to point out that due to the fact that I didn't compile any of your code, I had made a typo (leaving one of the 'i's out of "Visibility").

The while loop in question was probably in fact also related to that same function because I forgot to update the indexer 'i' being used for the loop iterations. I seemed to not care much for all the 'i's in that function.

In any case, I've updated the code above to fix these two specific issues. So, replace what you have entirely with the code from above, which has been modified.

However, if you do run into any more issues, you need to be a lot more specific about what is happening (especially if we had kept your original 2000+ lines of code!!).

When you're telling us that something went wrong, we need to know:

- What, if any, error message did you get. An exact copy of the error message is significantly more helpful to us than a paraphrasing is, so please try and write down the exact wording.

- If you got an error message, where did it occur? Tell not only the line in question, but the lines around it, what function it was in, etc. We need to know the context of the error, especially for something like a while loop hanging.

- If you don't get an error, what is the behaviour you are seeing? What is the behaviour you expect. Include as much detail here for best results.

When you say that "if you...click a wrong match then it seems to get confused," what do you mean by that??

The way it's working right now is this:

- If you haven't previously clicked on any mask, or if you have made a match (correct or otherwise) then graphic1 will be set to the graphic of the mask you clicked on, clickedMasks[0] will be set to the Object* of the mask you clicked on, and click1 will be set to 1 (one).

- If you have clicked on one mask since the last match (correct or otherwise), and you click on the same mask again, then it is treated the same as if you had not clicked on any mask. The values are the same as described above.

- Finally, if you've clicked on one mask object and then another mask object (and not clicked the same object twice), then graphic2 is set to the graphic of the mask you most recently clicked on, clickedMasks[1] will be set to the Object* of the mask you most recently clicked on, and click1 will be set to 2.

The clicks are checked in room_RepExec. If click1 has had its value set to 2, then it will compare the values of graphic1 and graphic2. If they are the same:

- counter is increased by 1 (one).
- click1 is set to 0 (zero).
- graphic1 and graphic2 are set to some junk values.

Otherwise, the values of these variables are unchanged, except if click1 is set to 2 then at the very end of the room_RepExec function the Clear function is called which:

- Sets click1 to 0 (zero).
- Sets clickedMasks[0] and clickedMasks[1] to null.

The logic here seems perfectly sound to me, so you need to explain what is happening, and what you expect to be happening. For example, are you expecting something to happen if the user selects an incorrect match? Because currently it's just being ignored..but that shouldn't "confuse" anything.