Input/suggestions on my code

3 posts

Flag Post

My game will have a simcity like map, and the player executes scavenge missions on ‘tiles’. 3 discovered items will be displayed, and the player then picks which ones to take.
Each tile has a limited amount of loot, of various types. My idea to simulate this is to have a ‘count’ for each category of loot, and a function will randomly determine the specific item.

So here’s what I have so far, just the scavenge function executed by a click of a button. No mission interface, or loot management. This code works, no errors, just doing this from scratch so looking for any input or advice.

public function scavenge(event:MouseEvent):void 
		 {
			 scavenging = true;
			 trace("scavenging commenced");
			 var lootInt = Math.round(Math.random()*10)
			 while (lootCount >0 && missedLoot <= 15)
			 {
			 	switch(scavenging)
			 	{
				case (lootInt >= 0 && lootInt <= 5  && resLoot >0):
					trace ("found food");
					resLoot = resLoot - 1;
					lootCount = lootCount -1;
					txtField.text = "found food";
				 	break;
				case(lootInt >= 6 && lootInt <=9 && comLoot >0):
					trace ("found gear");
					comLoot = comLoot - 1;
					lootCount = lootCount -1;
					txtField.text = "found gear";
					break;
				case (indLoot >0):
					trace ("found tools");
					indLoot = indLoot - 1;
					lootCount = lootCount -1;
					txtField.text = "found tools";
					break;
				default:
					trace ("found nothing");
					txtField.text = "found nothing";
					missedLoot = missedLoot +1;
					
				}
				
			 }
			 lootCount = 3;
			 missedLoot = 0;
			 scavenging = false;
		 }

Typical output is
“Found Food”
“Found Food”
“found Tools”

 
Flag Post

I’m not sure what the problem is. Does it compile? Does it run? Does it produce the output you want? Let me “hand-compile” this thing…

Info – Line 3: Assuming scavenging in a class-level variable.
Error – Line 5: missing a semicolon at the end.
Warning – Line 5: lootInt variable has no type.
Warning – Line 6: lootCount variable not initialized.
Warning – Line 6: missedLoot variable not initialized.
Info – Line 6: Assuming lootCount is a class-level variable.
Info – Line 6: Assuming missedLoot is a class-level variable.
Warning – Line 10: resLoot variable not initialized.
Info – Line 10: Assuming resLoot is a class-level variable.
Info – Line 14: Assuming txtField is an initialized class-level variable.
Warning – Line 16: comLoot variable not initialized.
Info – Line 16: Assuming comLoot is a class-level variable.
Warning – Line 22: indLoot variable not initialized.
Info – Line 22: Assuming indLoot is a class-level variable.
Psyduck – Line 36: Initialization section found at the end of the function body.

Overall, it should probably work reasonably well, although I have reservations about all the missing initializations. I am wondering what happens the first time you run this function since all the initialization is at the end. scavenging is a completely useless variable, set to true at the beginning, to false at the end, and the only place it’s used in-between is to set the switch to true. Just do switch(true) and remove all mention of scavenging.
x = x + 1; could be shortened to x += 1; and x = x – 1; to x -= 1; but that’s just for ease of reading (yea, I know it’s also a quarter of a rabbit’s fart faster, nobody cares.)

I would dissociate the scavenge function from the response to a MouseEvent. I understand that you likely have a big ‘scavenge’ button and you run this function when it’s clicked, but you’re taking a very dangerous shortcut by linking the two directly. Namely: you’re trusting the player. (gasp!)
Let me explain: Right now, as soon as the player clicks ‘scavenge’ they get to scavenge. What if they click when they were not supposed to? While their character is engaged in combat, for instance, or asleep, or unconscious, or while the game is paused? Right now you have no failsafe against that. What if you want to have a keyboard shortcut for scavenging? Are you going to duplicate the function as scavengeFromKeyboard(event:KeyboardEvent) just so you can accomodate an event you don’t even use in the body? You’d be much better off referring the Event to a checkForScavenge function which ensures that the conditions are right for scavenging and, if everything checks out, calls scavenge().

Magic numbers: Your loot table currenly has size 10, but the day you want to change that you’re going to be in trouble. Same with the attempt limit for finding anything (hard-coded 15) or the max number of loot items (hard-coded 3). These numbers which you have peppered throughout your code should be named constants. If there is any chance in the future that you could ever want to offer fewer or more than 3 loot items for the player to choose from, I’d turn lootCount into a parameter.

Hope this helps.

 
Flag Post

Thnx for the input.

Sorry, had a typo in there, there are no errors… I instantiated all the variables earlier in the main class, which currently has all the code (I’ll be separating things into classes as I expand).

I’ll definitely work on a separate “checkforscavenge” function.
I’ve studied from a book that had a state-based game framework, so that’s what I’m planning. This is a strategy game with little/nothing in real time, so it should fit well.