Help with selection error.

Started by Icey, Sun 11/11/2012 00:08:18

Previous topic - Next topic

Icey

It's a lot so I think it's better to show it like this cause it's a lot and I'm bad at explaining things separately. 

-.- .....come one not even one person can take a peek? No khris or Monkey :(


Lewis

"come one not even one person can take a peek? No khris or Monkey"

Maybe you could try to explain exactly what the problem is? At the moment I stare at that huge collection of screengrabs and go  8-0
Returning to AGS after a hiatus. Co-director of Richard & Alice and The Charnel House Trilogy.

Icey

Ok, I hoping that someone could have responded for that reason too.

I'm gonna try to explain this the best way I can. Basically, I have 4 Menus that are for the 4 characters in my game. Each character has 1 of the 4 menus that work sorta as their inventory menu.

In RPG's you have your menu that has options like
Attack
Magic
Item
etc

These options usually bring up another option menu which let you choose from a list of Spells and other items.

In my game, I changed it to that even though you have these options, you wont get another pop up menu where you can choose available commands but instead Custom Actions are called based on the setup of the Junctioning.

If you like at the in-game shot. Once you select a [command] such assault, your Ability inventory containing things such as Magic, items, etc... will pop up on the right under the label [Ability]

Next at the bottom right where you see [Charge] is the Junctioning name. When you select a Ability [Since Charge is going to be used as the example I'm gonna stick with the Assault abilities for now] like a Summon and Junction it to [Charge]. The Junctioning process is now complete and when Assault is called during battle, you will summon your Junctioned Summon.

Now here's the thing(hopefully you understand the Junction process now), let's say you try to Junction a Potion to Charge. Charge is only for Summons and Limit breaks and nothing more. So if you tried to Junction a Potion or Magic to Charge you would get a error saying "Can not be Junctioned to Charge" but the problem is, is that even if your Junctioning the right or wrong thing you still get not only one but all 5(4 if right) error messages. Only thing I can say that works right is the Junction process and the message stating that the Junction was successful but still right after that I get 4/5 error messages. The only one that doesn't show is the one that I was successful for.

Point is, I should only get one error message and if I'm successful then I shouldn't get one at all. :(

I hope that was clear cause I'm really bad at explaining things that are this big cause it's not really that complicated. I'm sure there's a way to go about doing this but I just can't grasp a thought.




Crimson Wizard

#3
Icey, although I seem to get the overall idea (vaguely), I can not comprehend situation in whole.
What I do understand, is that you have a whole lot of code, and you have errors in it. I do not think given explanations alone will make anyone understand what's going on there. Problems like this are solved by studying the larger part of code to get a grasp on it. Your screenshots provide few code excerpts which are nearly meaningless to everyone except you.

I may suggest to upload your full project so that people could examine it in detail. Unless you keep it as a strict secret, of course ;).

Other than that. I have a strong advice for you to learn using script modules and other programming techniques that help divide code in smaller parts. Judging on your screenshots, you have over 5000 lines in your script - that's pretty much even for "normal" programming language, and awful lot for AGS script.


//-------------------------------------------
EDIT:
Heh. After I wrote all that, I decided to look more just in case, and something got into my attention. I mean, I mostly tried to understand the code, but then I read comments more carefully.
Here's what you have:
Code: ags

if (ButtonXXX.NormalGraphic == YYY) // Check: Action
<...>
// However, if it's not on Action:
else
   Display("Cannot be junctioned to Action"); // Error

In other words, you detect that player tried to apply "junctioning" to something that is not Action but you display error telling that he can't use it on Action (although he doesn't).
Try to change error message to something like:
"Can be ONLY junctioned to Action".
And try if that works better...

Icey

Humm I think you sorta got part of it but it would be best if you did take a look at the game and anyone else if they want to.

http://www.mediafire.com/?w2t274lf7m2rxx5

Icey

Opps almost forgot, when the game loads and after you enter the pub. press Enter to open up the menu. Press Commands > Junction > Config Battle System. Click the new button box in the bottom right C. Now if you follow what I did in the picture up top then you will see the error.


Crimson Wizard

Ok, I'll maybe check that in couple of days.

Crimson Wizard

#7
Sorry for the big delay.
I finally got enough time to check that out. I did fix it, but before I give my solution I must say something.

Icey, I see that you really made big effort there, but I must speak frankly - you code is a terrible mess. You are putting too much features into game while not using proper code techniques. You probably may, eventually, finish this game, but it will become more and more difficult to work on it as you add more things in there.
I do not know if you wish to accept any advices, but I have a strong suggestion to you: put this game aside, split your project into separate parts and work them separately first, as a test games (like, junctioning system, etc). Secondly, apply more effort into improving your code style and techniques, otehrwise it will be like crafting a piece of art with a wrong tool (dumb analogy, but can't come with anything better).

And first of all, try to use proper code indentation. One of the mistakes you did there was made because you don't indent your code - so you could easily overlook that your if/else logic is done incorrectly.
Second thing - try to write simplier functions. If you see that function is becoming too long (like 3-4 screens), think about splitting it into logical parts. For example, in your case you could split the "Junctioning" function into 1) junction to Attack, 2) Junction to Cast, etc. This would also help you to see your mistake.




About my fixes. Firstly, here's the new function I made for getting name of action player is trying to junction ability to:
Code: ags

String GetActionNameFromButtonGraphic(Button *btn)
{
  if (btn.NormalGraphic == 194)
  {
    return "Attack";
  }
  else if (btn.NormalGraphic == 200)
  {
    return "Charge";
  }
  else if (btn.NormalGraphic == 853)
  {
    return "Use";
  }
  else if (btn.NormalGraphic == 862 ||
      btn.NormalGraphic == 863)
  {
    return "Cast";
  }
  
  // Put more variants here
  
  return "Unknown action";
}

It simply checks the button's graphic and deduces the name of action from it. I used the graphic numbers that I could quickly find in your code, if there are more variants, you may append them too.

Now, here's the function "Button185_OnClick" fixed:
Code: ags

function Button185_OnClick(GUIControl *control, MouseButton button){
	mouse.Mode = eModeInteract;
 
 // ------------------------- CHECK JUNCTION TO ATTACK -------------------------------
  if(Button184.NormalGraphic == 194){//Check: Attack
    if(player.ActiveInventory == Flamestrike){//Daves current item
      Button185.NormalGraphic = 857;//Change Junction button = Flamestrike
      DaveCom_1 = COM_flamestrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Froststrike){//Daves current item
      Button185.NormalGraphic = 858;//Change Junction button = Flamestrike
      DaveCom_1 = COM_froststrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Shockstrike){//Daves current item
      Button185.NormalGraphic = 859;//Change Junction button = Flamestrike
      DaveCom_1 = COM_shockstrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Windstrike){//Daves current item
      Button185.NormalGraphic = 860;//Change Junction button = Flamestrike
      DaveCom_1 = COM_windstrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Ultimate_weapon){//Daves current item
      Button185.NormalGraphic = 861;//Change Junction button = Flamestrike
      DaveCom_1 = COM_ultimateweapon;
      Display("Junctioned");//finished
    }
    else {//However if it's not on Attack
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
    return;
  }

  // ------------------------- CHECK JUNCTION TO CAST1 -------------------------------
  if(Button184.NormalGraphic == 862){//Check: Cast 1
    if(player.ActiveInventory == com4){//Daves current item
      Button185.NormalGraphic = 197;//Change Junction button = fire
      DaveCom_2 = COM_fire;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com5){//Daves current item
      Button185.NormalGraphic = 198;//Change Junction button = ice
      DaveCom_2 = COM_ice;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com6){//Daves current item
      Button185.NormalGraphic = 199;//Change Junction button = thunder
      DaveCom_2 = COM_shock;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com2){//Daves current item
      Button185.NormalGraphic = 195;//Change Junction button = Cure
      DaveCom_2 = COM_cure;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == fira){//Daves current item
      Button185.NormalGraphic = 864;//Change Junction button = 
      DaveCom_2 = COM_fira;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == firaga){//Daves current item
      Button185.NormalGraphic = 865;//Change Junction button = 
      DaveCom_2 = COM_firaga;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == icara){//Daves current item
      Button185.NormalGraphic = 866;//Change Junction button = 
      DaveCom_2 = COM_icara;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == icaga){//Daves current item
      Button185.NormalGraphic = 867;//Change Junction button = 
      DaveCom_2 = COM_icaga;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == thundara){//Daves current item
      Button185.NormalGraphic = 867;//Change Junction button = 
      DaveCom_2 = COM_shockara;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == thunaga){//Daves current item
      Button185.NormalGraphic = 868;//Change Junction button = 
      DaveCom_2 = COM_shockaga;
      Display("Junctioned");//finished
    }
    else {//However if it's not on Cast 0
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
    return;
  }

  // ------------------------- CHECK JUNCTION TO CAST2 -------------------------------
  if(Button184.NormalGraphic == 863){//Check: Cast 2
    if(player.ActiveInventory == com4){//Daves current item
      Button185.NormalGraphic = 197;//Change Junction button = fire
      DaveCom_3 = COM_fire;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com5){//Daves current item
      Button185.NormalGraphic = 198;//Change Junction button = ice
      DaveCom_3 = COM_ice;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com6){//Daves current item
      Button185.NormalGraphic = 199;//Change Junction button = thunder
      DaveCom_3 = COM_shock;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == com2){//Daves current item
      Button185.NormalGraphic = 195;//Change Junction button = Cure
      DaveCom_3 = COM_cure;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == fira){//Daves current item
      Button185.NormalGraphic = 864;//Change Junction button = 
      DaveCom_3 = COM_fira;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == firaga){//Daves current item
      Button185.NormalGraphic = 865;//Change Junction button = 
      DaveCom_3 = COM_firaga;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == icara){//Daves current item
      Button185.NormalGraphic = 866;//Change Junction button = 
      DaveCom_3 = COM_icara;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == icaga){//Daves current item
      Button185.NormalGraphic = 867;//Change Junction button = 
      DaveCom_3 = COM_icaga;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == thundara){//Daves current item
      Button185.NormalGraphic = 867;//Change Junction button = 
      DaveCom_3 = COM_shockara;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == thunaga){//Daves current item
      Button185.NormalGraphic = 868;//Change Junction button = 
      DaveCom_3 = COM_shockaga;
      Display("Junctioned");//finished
    }
    else {//However if it's not on Cast 0
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
    return;
  }

  // ------------------------- CHECK JUNCTION TO USE -------------------------------
  if(Button184.NormalGraphic == 853){//Check: USE
    if(player.ActiveInventory == potion){//Daves current item
      Button185.NormalGraphic = 323;//Change Junction button = Potion
      DaveCom_4 = COM_potion;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Hi_Potion){//Daves current item
      Button185.NormalGraphic = 324;//Change Junction button = Potion
      DaveCom_4 = COM_hipotion;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == ether){//Daves current item
      Button185.NormalGraphic = 325;//Change Junction button = Potion
      DaveCom_4 = COM_ether;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Elixer){//Daves current item
      Button185.NormalGraphic = 326;//Change Junction button = Potion
      DaveCom_4 = COM_elixer;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Phoenix_Down){//Daves current item
      Button185.NormalGraphic = 327;//Change Junction button = Potion
      DaveCom_4 = COM_phoenixpin;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Libra){//Daves current item
      Button185.NormalGraphic = 311;//Change Junction button = Potion
      DaveCom_4 = COM_libra;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Antidote){//Daves current item
      Button185.NormalGraphic = 328;//Change Junction button = Potion
      DaveCom_4 = COM_antidote;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Soft){//Daves current item
      Button185.NormalGraphic = 329;//Change Junction button = Potion
      DaveCom_4 = COM_soft;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Eye_Drops){//Daves current item
      Button185.NormalGraphic = 330;//Change Junction button = Potion
      DaveCom_4 = COM_eyedrops;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Echo_screen){//Daves current item
      Button185.NormalGraphic = 331;//Change Junction button = Potion
      DaveCom_4 = COM_echoscreen;
      Display("Junctioned");//finished
    }
    else {//However if it's not on USE
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
    return;
  }

// ------------------------- CHECK JUNCTION TO CHARGE -------------------------------
  if(Button184.NormalGraphic == 200){//Check: Charge
    if(player.ActiveInventory == Hellfire){//Daves current item
      Button185.NormalGraphic = 315;//Change Junction button = Hell fire
      DaveCom_5 = COM_ifrit;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Diamond_Dust){//Daves current item
      Button185.NormalGraphic = 316;//Change Junction button = Hell fire
      DaveCom_5 = COM_shiva;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Zantetsuken){//Daves current item
      Button185.NormalGraphic = 317;//Change Junction button = Hell fire
      DaveCom_5 = COM_odin;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Megaflare){//Daves current item
      Button185.NormalGraphic = 318;//Change Junction button = Hell fire
      DaveCom_5 = COM_bahamut;
      Display("Junctioned");//finished
      
    }
    else if(player.ActiveInventory == fierce_Bolt){//Daves current item
      Button185.NormalGraphic = 319;//Change Junction button = Hell fire
      DaveCom_5 = COM_ramuh;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Ori_slash){//Daves current item
      Button185.NormalGraphic = 320;//Change Junction button = Hell fire
      DaveCom_5 = COM_orion;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Gilgamesh){//Daves current item
      Button185.NormalGraphic = 321;//Change Junction button = Hell fire
      DaveCom_5 = COM_gilgamesh;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Heavenly_bless){//Daves current item
      Button185.NormalGraphic = 322;//Change Junction button = Hell fire
      DaveCom_5 = COM_havenbird;
      Display("Junctioned");//finished
    }
    else {//However if it's not on Charge
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
    return;
  }
}


What was your mistake there?
You put several sequential "if" blocks, making program check them ALL, even if previous checks were successfull. Because of this, if you applied ability to Attack, and that ability got junctioned, the program still went to another check, like Cast, failed, and displayed "error" message.
So, I put "return" statements in each "if" block to prevent this from happening.
I also used my new function to show real current action name in the error message:
Code: ags

Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error

Icey

Thanks man, my plan right now is just to get this game out the way where it's playable and fun. So it's that I don't want to take your advice, it's just I can't take it just yet ;). Someday I plan to port the game to another game engine like Unity. But before I do that I do wanna make a few test demos to test features for current and future games so I won't have so much of a problem to coding them and fixing them. :)

Now with the code, I thought it was checking it all but the sad part is I never really use Return since I never knew how to use it. Also should I place the first code in the general script or in a new script?

Crimson Wizard

#9
Quote from: icey games on Sat 15/12/2012 18:47:24
Now with the code, I thought it was checking it all but the sad part is I never really use Return since I never knew how to use it.
'Return' simply exists the function at this point. Every function has at least one "return" in the end, even if you don't write it yourself.
'Return' returns back some value (or nothing). For example in my function it returns the string.
It works very simple:
Code: ags

int return_some_value()
{
  return 10;
}

This function will return number 10:
Code: ags

int a = return_some_value();

This line will call function, return back a number and store it in "a" variable.

More on this in the manual: http://www.adventuregamestudio.co.uk/wiki/?title=Scripting_tutorial_part_2#Our_own_function
This is actually quite important, because programming is all about writing functions, and returning values back is very useful.

If you are making number of checks, where only one check may succeed, then you may use return to drop from function and not do anything that is not needed:
Code: ags

function DoSomething()
{
  if (Button.Graphic == 100)
  {
   // do something
   return; // no need to go further
  }

  if (Button.Graphic == 200)
  {
   // do something
   return; // no need to go further
  }

  // etc
}


But in that case you could actually solve the problem without return, by using 'if/else if' structure:
Code: ags

function DoSomething()
{
  if (Button.Graphic == 100)
  {
   // do something
  }
  else if (Button.Graphic == 200)
  {
   // do something
  }
  else
  {
    // if nothing else matches, do other thing
  }
}

In this case only one of those blocks will be executed anytime the function is called.

I used 'return' for no certain reason, actually; I could use 'else ifs' instead. It's just that I belive that function is too long and could be divided into parts, each part handling one action check. For example, you could put "Attack" check into separate function:
Code: ags

function TryJunctionWithAttack()
{
    if(player.ActiveInventory == Flamestrike){//Daves current item
      Button185.NormalGraphic = 857;//Change Junction button = Flamestrike
      DaveCom_1 = COM_flamestrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Froststrike){//Daves current item
      Button185.NormalGraphic = 858;//Change Junction button = Flamestrike
      DaveCom_1 = COM_froststrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Shockstrike){//Daves current item
      Button185.NormalGraphic = 859;//Change Junction button = Flamestrike
      DaveCom_1 = COM_shockstrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Windstrike){//Daves current item
      Button185.NormalGraphic = 860;//Change Junction button = Flamestrike
      DaveCom_1 = COM_windstrike;
      Display("Junctioned");//finished
    }
    else if(player.ActiveInventory == Ultimate_weapon){//Daves current item
      Button185.NormalGraphic = 861;//Change Junction button = Flamestrike
      DaveCom_1 = COM_ultimateweapon;
      Display("Junctioned");//finished
    }
    else {//However if it's not on Attack
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
    }
}


And then you call these functions from central function:
Code: ags

function Button185_OnClick(GUIControl *control, MouseButton button){
  mouse.Mode = eModeInteract;
 
  if(Button184.NormalGraphic == 194){//Check: Attack
     TryJunctionWithAttack();
  }
  else if(Button184.NormalGraphic == 862){//Check: Cast 1
     TryJunctionWithCast1();
  }
  // etc
}

This makes function smaller, and easier to understand. Also whole logic is more explicit, you read it as a list of actions.




Quote from: icey games on Sat 15/12/2012 18:47:24
Also should I place the first code in the general script or in a new script?
You may put it right before Button185_OnClick function, since it is the only function that uses it at the moment. If you'd want to use it somewhere else, make sure this new function is put before that place.



//-----------------------
EDIT:
I should perhaps add more.
Try to make things simplier. If you see that you are repeating some action several times, you are probably no doing something right. For example, in your case, there's this "Display("Junctioned");" everywhere. The function could be reorganized to make it appear only once, for instance:

Code: ags

function TryJunctionWithAttack()
{
    if(player.ActiveInventory == Flamestrike){//Daves current item
      Button185.NormalGraphic = 857;//Change Junction button = Flamestrike
      DaveCom_1 = COM_flamestrike;
    }
    else if(player.ActiveInventory == Froststrike){//Daves current item
      Button185.NormalGraphic = 858;//Change Junction button = Flamestrike
      DaveCom_1 = COM_froststrike;
    }
    else if(player.ActiveInventory == Shockstrike){//Daves current item
      Button185.NormalGraphic = 859;//Change Junction button = Flamestrike
      DaveCom_1 = COM_shockstrike;
    }
    else if(player.ActiveInventory == Windstrike){//Daves current item
      Button185.NormalGraphic = 860;//Change Junction button = Flamestrike
      DaveCom_1 = COM_windstrike;
    }
    else if(player.ActiveInventory == Ultimate_weapon){//Daves current item
      Button185.NormalGraphic = 861;//Change Junction button = Flamestrike
      DaveCom_1 = COM_ultimateweapon;
    }
    else {//However if it's not on Attack
      Display("Can not be junctioned to %s", GetActionNameFromButtonGraphic(Button184));//Error
      return;
    }
    
    Display("Junctioned");//finished
}

What I did here - I moved "Display("Junctioned")" piece to the end of function, so it will appear in any case. But we don't want it appear if there was an error, right? So, to prevent this, I put "return" in the final "else" block.
Now, if that was correct ability, it will pass to corresponding block, then display "Junctioned" message. If it is incorrect ability, it will get into "else" block, display "Can't be junctioned" and exit function without displaying "Junctioned" message.

Icey

huh, it's a lot but it makes sense, it's just seems to be the thing I just need to play around with (not the your fixed code but something different, you know.)
But still, Thanks a lot, Not only did you just help with me that staggering problem but you also showed me how to use the Return.

SMF spam blocked by CleanTalk