SOLVED: Conditions problem. script continues for another room.

[barefoot]

Hi

Sorry guys,

Something in the below script is not acting as is planned. Its from the Global script for when an inventory item is clicked on the player (cwizard)..

The part that's giving me gyp is when the book or horn are used in room 6 on the player.
It runs as normal but at the end of that particular piece of script it carries on as if player is in room 9.. (about using the the mirror)

Obviously conditions need to apply depending on what room the player is in when using inventory items on himself.

I know its a conditional thing and i have tried amending but have as yet not found the solution. I shall study it again but in the meantime if anyone can advice i would be greatfull.

I hope i have included enough info and that you'll excuse me.

Code: ags

function cwizard_UseInv()
   
{    
  if (HasPlayerBeenInRoom(3)&& player.ActiveInventory==ihorn){ 
  
  player.ActiveInventory=null;
  cwizard.LoseInventory(ihorn);
  player.FaceObject(object[3]);
  Display("You take out your magic Horn and blow with all of your might! The wailing banshee sound is unbelievably wailing so you close your ears.");
  aBird.Play();
  object[3].Move(20, 13, -1, eBlock, eAnywhere);
  object[3].Visible = false;  
  aBird.Stop();
  cwizard.Say("Good job brought my horn along!");
  cwizard.Say("I need to get that prism!");
  cwizard.Clickable=0;
  
} 

 else if (HasPlayerBeenInRoom(3)&& player.ActiveInventory==ibook && !player.HasInventory(imirror)) {[/b]
   
   player.ActiveInventory=null;
   cwizard.LoseInventory(ibook);
   player.FaceObject(object[3]);
   cwizard.Say("Now let me see. Ah, here's a spell of bat calling. If there is one thing a great Macaba bird is frieghtened of it's a bat!!");
   Display("You utter a few magic words");
   cwizard.Say("O magi karindo marlon ellami!");
   cwizard.AddInventory(ibook);
   object[4].Visible = true;
   aBird2.Play();
   object[4].Move(498, 76, -1, eNoBlock, eAnywhere);
   object[3].Move(20, 13, -1, eNoBlock, eAnywhere);
   object[4].Move(20, 20, -1, eBlock, eAnywhere);
   object[4].Visible = false;
   object[3].Visible = false;
   Display("The Macaba bird flys off in distress!");
   aBird2.Stop();
   cwizard.Say("Now to get that prism!");
   cwizard.Say("I need to get it down from that high branch!");
  
   
   
 }
  else if (HasPlayerBeenInRoom(7)&& player.ActiveInventory==ibook && player.HasInventory(imirror)) {
   
   
   Display("You don't need anything from your magic spell book just now");
  
  
    }

  else if (HasPlayerBeenInRoom(7)&& player.ActiveInventory==ibook && !player.HasInventory(imirror)) {
   
   
   Display("You may need to cast a spell on something else!");
  
  }

  else if (HasPlayerBeenInRoom(8)&&  player.FaceLocation(30, 307)&& player.ActiveInventory==imirror && player.HasInventory(imirror)) {
  
  SetTimer(1, 0);
    
    }
    
     cwizard.ChangeView(35);
     Display("You take out the mirror so you can see as you walk backwards towards the statues.");
     Display("You hope it's right what is said about using a mirror against such an diabolical creature!!");
     caliska.SayAt(350, 100, 270, "Come closer. Look into my eyes. Look deep deep into my eyes.");
     cwizard.Walk(459,213, eBlock); 
     Display("You manage through the mirror to stop at the Urn of Osisi and grab the prism");
     cwizard.AddInventory(iblueprism);
     
     GiveScore(1);
     SetGlobalInt (1, 100);
     
     cwizard.Say("Got it! Now to try and sort that creature out!");
     cwizard.Say("Hopefully my magical Staff will stop that monster and banish it to the burning fires of hell!!!");
     Display("You get out your magical Staff and bang it on the ground");
     cwizard.ChangeView(37);
     cwizard.Animate(0, 5);
     cwizard.ChangeView(3);
     object[8].Visible = true;
    
     Wait(70);
     
     caliska.SayAt(350, 100, 270, "Aghhhhhhhhhhh.");
     object[8].Visible = false;
     object[0].Move(490, 352, 8, eBlock, eAnywhere);
     cwizard.Say("It's gone back to hell! That should relieve the folk of Latvik!");
     Display("You shout to the village folk that the monster has gone. They sing with rejoice!");
     cwizard.Say("Now, I really must get to another prism. I remember someone saying that had seen a sage at the Narvik staits");
     Display("You head off to the Narvik staits");
     cwizard.IgnoreWalkbehinds = 1;
     RemoveWalkableArea(2);
     cwizard.Walk(620, 244, eBlock, eAnywhere);
}   

cheers

barefoot

[monkey0506]

Okay, without looking too much into it..you're never checking which room the player is currently in, but only which rooms he has previously visited..so when you say that it's giving you problems in a particular room..I think that in itself might be causing you some issues.

That, and again noting that I didn't read too deeply into your code, and I also noticed that you're doing this:

Code: ags

cwizard.ChangeView(35);
     Display("You take out the mirror so you can see as you walk backwards towards the statues.");
     Display("You hope it's right what is said about using a mirror against such an diabolical creature!!");
     caliska.SayAt(350, 100, 270, "Come closer. Look into my eyes. Look deep deep into my eyes.");
     cwizard.Walk(459,213, eBlock); 
     Display("You manage through the mirror to stop at the Urn of Osisi and grab the prism");
     cwizard.AddInventory(iblueprism);
     
     GiveScore(1);
     SetGlobalInt (1, 100);
     
     cwizard.Say("Got it! Now to try and sort that creature out!");
     cwizard.Say("Hopefully my magical Staff will stop that monster and banish it to the burning fires of hell!!!");
     Display("You get out your magical Staff and bang it on the ground");
     cwizard.ChangeView(37);
     cwizard.Animate(0, 5);
     cwizard.ChangeView(3);
     object[8].Visible = true;
    
     Wait(70);
     
     caliska.SayAt(350, 100, 270, "Aghhhhhhhhhhh.");
     object[8].Visible = false;
     object[0].Move(490, 352, 8, eBlock, eAnywhere);
     cwizard.Say("It's gone back to hell! That should relieve the folk of Latvik!");
     Display("You shout to the village folk that the monster has gone. They sing with rejoice!");
     cwizard.Say("Now, I really must get to another prism. I remember someone saying that had seen a sage at the Narvik staits");
     Display("You head off to the Narvik staits");
     cwizard.IgnoreWalkbehinds = 1;
     RemoveWalkableArea(2);
     cwizard.Walk(620, 244, eBlock, eAnywhere);

Every single time the player uses any inventory item at all ever on the wizard. Is this really expected behaviour? Well, I mean, at least I think that it's being run every time. Your indentation "style" continues to deter me from actually checking, I'm just basing this off the fact that there's only one closing brace after all that code.

Looking over the code snippet that I'm guessing gets run for any inventory item, any time it is used on the wizard, I really find it interesting that you wouldn't even check if the player has the mirror before you let them use it.

Edit: I just read over the prelude text of your original post, and I really, really have to ask whether or not English is your native tongue. Because as I already pointed out above you are never checking which room the player is in, yet it would appear from what you've said that you think that this is exactly what you are doing. How you would conceive that "HasPlayerBeenInRoom" is synonymous with "IsPlayerInRoom" completely eludes me unless you are not a native English speaker. You've never, to my knowledge, given any indication that English is not your native language, but seriously, this makes me wonder.

[barefoot]

Hi

sorry of it deters you monkey, but with less than 50% vision i have to do it this way, i find i can see it better.

I amended a line to read:

Code: ags

 if (player.PreviousRoom==3 && player.ActiveInventory==ibook) {

Surely this piece of script should only run if player's previous room was 3 and end as it should... but it continues on as if room 9 as if he has used the mirror...



barefoot

[monkey0506]

You posted while I was editing, so I'm going to post this again in the hopes that I might actually get a response (seeing as you blatantly ignored the things I mentioned in my prior post, I don't have high hopes for this):

I just read over the prelude text of your original post, and I really, really have to ask whether or not English is your native tongue. Because as I already pointed out above you are never checking which room the player is in, yet it would appear from what you've said that you think that this is exactly what you are doing. How you would conceive that "HasPlayerBeenInRoom" is synonymous with "IsPlayerInRoom" completely eludes me unless you are not a native English speaker. You've never, to my knowledge, given any indication that English is not your native language, but seriously, this makes me wonder.

That being said, you still are NOT checking what room the player is currently in..EVER. To do that you would need to do this:

Code: ags

if (player.Room == 5) {

Edit: And at the sake of you missing it again, did you read the part where I mention that the code snippet I took from yours is not encased inside of any condition, and therefore gets run any time the player uses any inventory item in any room under any conditions for any reason at any time whatsoever on the wizard regardless of other assumed prerequisites, without limitation?!?

Oh, and I mean absolutely no offense to any medical conditions you may have regarding your vision, but there is absolutely no way in hell that I will believe that this:

Code: ags

if (blah) {
   // do something
                    }
                                        else if (blah blah)    {
 }
         else
          {
if (this)
{
    // ..
                     }
                                                 }

Is more readable than this:

Code: ags

if (blah) {
  // do something
}
else if (blah blah) {
}
else {
  if (this) {
    // ...
  }
}

I don't care if you want to leave blank lines between bits of your code, but saying that raping the hell out of the indentation which is AUTOMATICALLY handled for you helps you to be able to see it better is a blatant lie. Period.

..I realize that this example is very extreme. The code you've posted here is much, much neater than some of your prior posts. But the fact remains that you are going out of your way to effect this..which bothers me quite a bit because it makes your code very much less readable.

[barefoot]

Hi Monkey

i knew about hasbeen andprevious but did not find player.Room anywhere..

I thank you for that statement of player.room.

if (player.Room == 6 && player.ActiveInventory==ibook) {

I have looked again at the script and have now got it functioning as it should..
so I thank you for that monkey..

I did not ignore you post... i was in AGS..

I am upset a little by your remarks. Its all so obvious to you. I do respect you for your script knowledge but maybe you could ease up a little..

I've done teaching myself, guitar, I know some students can be a pain.. but you have to have rock hard patience...

regards
barefoot

[Khris]

You can't find player.Room in the manual. What you have to do is realize that player is pointing to a character (here: cWizard) and that you have to look at the Scripting -> Character section to find out about available functions and properties.
That's where you'll find Character.Room.

As far as I can see, monkey got (understandably) worked up because you told him your screwed up indentation style is seen better by you than a clear, structured, consistent indentation which was recommended to use by us several times. That's clearly bullshit, even if your vision were at 10%. Somebody who can't read a map isn't gonna have an easier time finding their way in an oriental historic city center.

On to your code:
Stripped of all the specific commands, with consistent indentation, you have this:

Code: ags

function cwizard_UseInv() {
    
  if (HasPlayerBeenInRoom(3)&& player.ActiveInventory==ihorn) {

    // CODE
    
  } 

  else if (HasPlayerBeenInRoom(3)&& player.ActiveInventory==ibook && !player.HasInventory(imirror)) {  
    
    // CODE
   
  }

  else if (HasPlayerBeenInRoom(7)&& player.ActiveInventory==ibook && player.HasInventory(imirror)) {
      
    Display("You don't need anything from your magic spell book just now");
  
  }

  else if (HasPlayerBeenInRoom(7)&& player.ActiveInventory==ibook && !player.HasInventory(imirror)) {
      
    Display("You may need to cast a spell on something else!");
  
  }

  else if (HasPlayerBeenInRoom(8)&&  player.FaceLocation(30, 307)&& player.ActiveInventory==imirror && player.HasInventory(imirror)) {
  
    SetTimer(1, 0);
    
  }

  // CODE that is run every time the player uses inventory on themselves (there probably shouldn't be any)
    
}

1: You can group similar conditions.

2: I guess you meant to use "player.Room" but didn't know the command. So I'll use that. The point still stands, why did you not ask about how to find out the current room? Why use something that's clearly wrong instead?

3: player.FaceLocation(30, 307) will return some arbitrary value and isn't supposed to be part of a condition. To check if the player is facing a certain direction/location, check their .Loop property (and their position).

4: if the player's ActiveInventory is X, he must have X in his inventory so there's no need to separately check the latter.

[Khris]

[Put this in a second post to avoid tl;dr.]


Here's how I would code the function:

Code: ags

function cwizard_UseInv() {

  InventoryItem*ai = player.ActiveInventory; // store it in a pointer to avoid typing it all the time

  if (ai == ihorn) {
  
    // room 3's name
    if (player.Room == 3) {

      // player uses horn in room 3
      player.ActiveInventory=null;
      cwizard.LoseInventory(ihorn);
      player.FaceObject(object[3]);
      Display("You take out your magic Horn and blow with all of your might! The wailing banshee sound is unbelievably wailing so you close your ears.");
      aBird.Play();
      object[3].Move(20, 13, -1, eBlock, eAnywhere);
      object[3].Visible = false;  
      aBird.Stop();
      cwizard.Say("Good job brought my horn along!");
      cwizard.Say("I need to get that prism!");
      cwizard.Clickable=0;
    }

    // any other room
    else Display("There's no point in using the horn here.");
  }


  else if (ai == ibook) {

    // room 3
    if (player.Room == 3) {
 
      // player uses book in room 3

      if (player.HasInventory(imirror)) {

        // player uses book in room 3 while having the mirror in their possession
      }
      else {

        // player uses book in room 3 while NOT having the mirror in their possession
      }
    }

    // room 7
    else if (player.Room == 7) {

      // player uses book in room 7

      if (player.HasInventory(imirror)) {

        // player uses book in room 7 while having the mirror in their possession
        Display("You don't need anything from your magic spell book just now");
      }
      else {

        // player uses book in room 7 while NOT having the mirror in their possession
        Display("You may need to cast a spell on something else!");
      }
    }

    else if (player.Room == 8) {

      // player uses book in room 8

      if (player.HasInventory(imirror)) {

        // player uses book in room 8 while having the mirror in their possession
      }
      else {

        // player uses book in room 8 while NOT having the mirror in their possession
      }
    }

    // any other room
    else Display("I don't need to use the spell book here.");
  }


  else if (ai == imirror) {

    if (player.Room == 8) {

      // player uses mirror in room 8
    }

    // any other room    
    else Display("What would be accomplished by that?");
  }


  // any other item
  else Display(String.Format("There's no point in using the %s that way.", ai.Name));    
}

The comments are property indented everywhere, indicating where code lines should start at.
I usually put a blank line before indented blocks of code to separate them vertically from the if line. The closing bracket is the only character in its line, so no blank line under the block.

I also noted that you're constantly alternating between using player and cwizard. As long as the player isn't going to control another character at some point during the game, those will remain the same and are thus interchangeable. You seem to know this; I wonder why you aren't sticking to one of the two then (preferably player)?

EDIT: Added "else" to the Display line at the very end.

[barefoot]

Hi

Thank you so much Khris... and of course monkey...

my indentation winds people up so i guess i'll use as it should be and hope i can read it properly... i apologise..

Khris: You put time and effort into telling me what i should have used etc and so did monkey in a way but i guess your reply was more user friendly..

I did manage it on my own with the playerRoom== and your message was more than welcomed.

Thank you again

barefoot

[Khris]

You're welcome!

As a final point about the indentation: what winds us up is not that we don't like your style, what winds us up is that it is one of the (if not the) primary reason(s) for the errors that caused this thread and it wasn't the first time this happened.
Proper indentation greatly helps avoiding if - else if - bracket placement mistakes.

Also, if one were to write nested if else blocks from top to bottom without going back to change something, one would never have to manually indent anything at all; AGS would always put the cursor in the correct spot after pressing enter. It even moves the } to the left where it belongs, that's how great a job it does at that. Who are you to interfere?

[monkey0506]

barefoot, I apologize if I got a bit brash there. Khris' reiteration of my points is appreciated, particularly because he kept his cool a bit better than myself.

I would like to point out that you never gave any indication that you had read the portion of my post where I told you what the problem was. That was part of the reason why I got so heated. I told you, rather blatantly I thought, where the problem was, and you didn't respond to it at all.

You did respond to the issue of checking which room the player is in after I told you specifically what to use to do so. I understand if you don't always know how to use or find every function/property, but you should be capable of thinking things through logically. A function called "HasPlayerBeenInRoom" is obviously not the way to check whether the player is currently in that room, but simply whether they have ever been in said room. You did manage to find the Character.PreviousRoom property (somehow, you didn't specify), but again, that, obviously, would only tell you which room was the player's previous room. If you then took the search a bit further, you could perhaps look up the PreviousRoom property in the manual. And what do you know? The Character.Room property is the very next entry on the same page!

I honestly don't mean to offend you, or insult you in any way, but time and time again we have tried to help you learn how to help yourself, and you consistently continue to make the same mistakes over and over. This is why I said what I said. I really am sorry for the tone that I took, but when I provide you with an answer and you completely fail to respond (and indeed continue saying that the problem still exists), you must realize how frustrating that is.

[barefoot]

Hi Monkey

I am not offended by your remarks, just taken back a bit. I'm ok and appreciate all your posts and I hope that you can be on hand in any further issues I have.

You guys have so much to give and some of us appreciate that fact.

I know I can be a pain but I mean well as no doubt you do..

I don't just post and sit here waiting for someone to give me the answer, I keep on looking for myself and if i sort it then i say so...

Thank you monkey and of course khris...  

barefoot