Closeting RPPs... (verification needed on code snippet)

[muffins]

I'm working on a pretty big script at the moment, but it's not quite far along enough yet to post, so I'm just going to ask this here instead of posting a potentially very buggy script in the In-Progress section...

The main thing I want to check is, as usual, whether I've done something right or wrong with a portion of the script.

Here it is:

void closet_red_pixel_potion() {
if(closet_amount($item[red pixel potion]) < 4 && item_amount($item[red pixel potion]) >= 4) { 
	put_closet((4 - closet_amount($item[red pixel potion])), $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 3 && (closet_amount($item[black pixel]) >= 3 || item_amount($item[black pixel]) >= 3) && (closet_amount($item[red pixel]) >= 2 || item_amount($item[red pixel]) >= 2)) { 
	create(1, $item[red pixel potion]); 
	put_closet(1, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 2 && (closet_amount($item[black pixel]) >= 6 || item_amount($item[black pixel]) >= 6) && (closet_amount($item[red pixel]) >= 4 || item_amount($item[red pixel]) >= 4)) { 
	create(2, $item[red pixel potion]); 
	put_closet(2, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 1 && (closet_amount($item[black pixel]) >= 9 || item_amount($item[black pixel]) >= 9) && (closet_amount($item[red pixel]) >= 6 || item_amount($item[red pixel]) >= 6)) { 
	create(3, $item[red pixel potion]); 
	put_closet(3, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 0 && (closet_amount($item[black pixel]) >= 12 || item_amount($item[black pixel]) >= 12) && (closet_amount($item[red pixel]) >= 8 || item_amount($item[red pixel]) >= 8)) { 
	create(4, $item[red pixel potion]); 
	put_closet(4, $item[red pixel potion]); 
}
}

I don't have a character with any pixels or a need for any potions at the moment, so I can't test it out properly, but is there anyone who can just take a look at this and let me know if it'll do what I'm wanting it to do?

The main thing that I'm hoping will work are the or statements within the and statements.

 

[Nightmist]

I assume this script is ment to put RRP's into the closet to atleast have 4 in there and if there is already over 4 in there then just ignore the extras.

Suggestion:

int creatable_amount( item it )

as a replacement for the massive or//and. (Speaking of which the posted snippet should work, I haven't exactly tested it in-game but from looking at it, it seems okay)

 

[muffins]

Yup, that's what it's meant to do.

Of course! I'd forgotten about that...

So, it'd just be

void closet_red_pixel_potion() {
if(closet_amount($item[red pixel potion]) < 4 && item_amount($item[red pixel potion]) >= 4) { 
	put_closet((4 - closet_amount($item[red pixel potion])), $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 3 && creatable_amount($item[red pixel potion]) >= 1) { 
	create(1, $item[red pixel potion]); 
	put_closet(1, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 2 && creatable_amount($item[red pixel potion]) >= 2) { 
	create(2, $item[red pixel potion]); 
	put_closet(2, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 1 && creatable_amount($item[red pixel potion]) >= 3) { 
	create(3, $item[red pixel potion]); 
	put_closet(3, $item[red pixel potion]); 
}
if(closet_amount($item[red pixel potion]) == 0 && creatable_amount($item[red pixel potion]) >= 4) { 
	create(4, $item[red pixel potion]); 
	put_closet(4, $item[red pixel potion]); 
}
}

?

That would definitely be much less of a headache (not to mention a timesaver)... if anyone can test it out and let me know if it works, it'd be much appreciated.

Also, would it be "safer" to do it the way I have it here, or to use else ifs... or would that not make a difference? (Sorry, brain is borked due to actual headache, which may or may not have been caused by writing the original version of this, heh)

Edit: I just realized this could even be simplified with a while loop... at least I think so...

void closet_red_pixel_potion() {
if(closet_amount($item[red pixel potion]) < 4 && item_amount($item[red pixel potion]) >= 4) { 
	put_closet((4 - closet_amount($item[red pixel potion])), $item[red pixel potion]); 
}

if(closet_amount($item[red pixel potion]) < 4 && creatable_amount($item[red pixel potion]) >= 1) {
	while(closet_amount($item[red pixel potion]) < 4 && creatable_amount($item[red pixel potion]) >= 1) {
		create(1, $item[red pixel potion]); 
		put_closet(1, $item[red pixel potion]); 
	}
}
}

 

[Nightmist]

[quote author=muffins link=topic=641.msg3018#msg3018 date=1166316189]
Edit: I just realized this could even be simplified with a while loop... at least I think so...
[/quote]
I would prefer you used the other version, sure it's bigger but its less server intensive (Instead of making 1 at a time it makes the required amount)

Perhaps something like this

void closet_red_pixel_potion()
{
 if( closet_amount( $item[red pixel potion]) < 4)
 {
  int MoveAmount = 4-closet_amount( $item[red pixel potion]);
  if( item_amount( $item[red pixel potion]) < MoveAmount && creatable_amount( $item[red pixel potion]) >= MoveAmount)
  {
   create( MoveAmount, $item[red pixel potion]);
  }
  if( item_amount( $item[red pixel potion]) >= MoveAmount)
  {
   put_closet( MoveAmount, $item[red pixel potion]); 
  }
  else
  {
   print( "Unable to move required amount ("+MoveAmount+") of red pixel potions to closet.");
  }
 }
}

Yeah you can kill some of the "{" and "}" I used in there but its just a habit for me to write them in even if they don't "need" to be. This hits the server once to create and then once to closet. (Assuming it works, which it might not, who knows. In other words, never tested.)

And no I don't have a file to attach with that in it because I just typed it up in here >>.

Edit: Okay some fixage... How this one works is find out if you even need any more in the closet, then if you do need more, how many need to be moved ("MoveAmount") and then if that amount is higher then the current number of potions of have on hand then it creates the necessary amount if you have the pixels to do so. It then moves that amount to the closet, or if it is unable to move the required amount it does the abort and error message.

 

[Metraxis]

Alternatively, you could do something like this:

int max (int a, int b) {
  if( a > b ) { return a; } else { return b; }
}

int min (int a, int b) {
  if( a > b ) { return b; } else { return a; }
}

void closet_red_pixel_potions() {
  if(!have_skill($skill[Ambidextrous Funkslinging])) {
    create(max(0,min(creatable_amount($item[red pixel potion]),4 - closet_amount($item[red pixel potion]) - item_amount($item[red pixel potion]))),$item[red pixel potion]);
    put_closet(max(0,min(item_amount($item[red pixel potion]), 4 - closet_amount($item[red pixel potion]))),$item[red pixel potion]);
  }
}

 

[muffins]

You guys are gonna make my brain explode.

[mini-rant about my stubborness regarding code]

Aww, I like my dinosaur code. I'd probably like the other, more efficient, methods if I understood them, but sadly, I don't... well, not right now, anyway. The last method posted does look nice and efficient, and I do somewhat understand it, but I'm not one to implement code I don't understand, if only because I may reopen the script at a later date and wonder what the hell I was trying to accomplish there, even if I do heavily comment it (which I do, usually).

I suppose you could say that I treat most programming "languages" like I would any other form of language, and therefore am probably the person most likely to speak "broken english" in this analogy... but, if it makes sense to me, it still makes sense to the "people I'm talking to", and I'm likely to butcher the "correct" use, then why change?

[/mini-rant about my stubborness regarding code]

That being said, please don't mistake my stubborness for ungratefulness. I do appreciate all the help that's been offered here!

Also, in case anyone was wondering and hadn't already seen it yet, this was a snippet from a script I was in the middle of writing.

Check it out here and let me know what you think: ClosetQuest

 

[Metraxis]

[quote author=muffins link=topic=641.msg3078#msg3078 date=1166603366]...I'm not one to implement code I don't understand, if only because I may reopen the script at a later date and wonder what the hell I was trying to accomplish there, even if I do heavily comment it (which I do, usually).
[/quote]

Fair enough. Here's how it works. max() and min() are the classical functions of the type, and are so eminently useful that I'm surprised they aren't native to ASH. Respectively, they return the larger and smaller of the two arguments. ASH should never see a situation where !> is not equivalent to <=, so the use of the second parameter in a tie should be excuable.

Next, the if(): If you have Funk in both your left hand and your right, you don't need RPPs for any Quest-related purpose, so we skip the whole shooting match.

Next, the create line: We want to create all the potions in one go. We need 4 total, but we can count those in the closet and those in inventory against that 4 {Hence 4 - closet_amount - item_amount()}. We like holawutol, so we use the min() and creatable_amount() functions to ensure that we don't ask Mafia to make more potions than we have pixels for. Similarly, we may have been a pixel-grabbing freak and have dozens of RPPs laying around, so we include the max(0,...) to ensure that we don't try to create a negative number of an item.

Finally, the put_closet() line. We try to put in 4, less what we have in the closet already, but no fewer than 0, much like the line above.