I am trying to make it so that by default, a person can learn new moves by leveling. But, they can only have a max of 3 moves, and if they want to learn different ones, they have to get rid of an older move. Here is what I have. The problem, however, is that I can use the first two moves in the selection, but not the third for some reason.
mob/verb
First()
set category = null
for(var/mob/Pet/M in world)
if(M.owner == usr&&M.out)
usr.first = 1
usr.second = 0
usr.third = 0
usr.fourth = 0
M.Move1 = M.Attacks[1]
usr<< "You switched to [M.Move1]."
mob/verb
Second()
set category = null
usr.second = 1
for(var/mob/Pet/M in world)
if(M.owner == usr&&M.out)
usr.first = 0
usr.second = 1
usr.third = 0
usr.fourth = 0
M.Move2 = M.Attacks[2]
usr<< "You switched to [M.Move2]."
Third()
set category = null
for(var/mob/Pet/M in world)
if(M.owner == usr&&M.out)
usr.first = 0
usr.second = 0
usr.third = 1
usr.fourth = 0
M.Move3 = M.Attacks[3]
usr<< "You switched to [M.Move3]."
//These all are for choosing your move.
mob/proc
Usemove()
if(usr.first)
for(var/mob/Pet/M in world)
if(M.owner == usr)
if(M.Attacks[1] == "Slash")
M.Slash()
if(M.Attacks[1] == "Stab")
M.Stab()
if(M.Attacks[1] == "Guard")
M.Guard()
if(usr.second)
for(var/mob/Pet/M in world)
if(M.owner == usr)
if(M.Attacks[2] == "Slash")
M.Slash()
if(M.Attacks[2] == "Stab")
M.Stab()
if(M.Attacks[2] == "Guard")
M.Guard()
if(usr.third)
for(var/mob/Pet/M in world)
if(M.owner == usr)
if(M.Attacks[3] == "Slash")
M.Slash()
if(M.Attacks[3] == "Stab")
M.Stab()
if(M.Attacks[3] == "Guard")
M.Guard()
//These are to use the move, and the corresponding procs.
The first two segments in the Usemove() proc work. But, for some reason, the third doesn't work. I have two theories as to why it isn't working, and that is what I want everyones opinions on. Here they are:
1. I added the move to the list wrong, and need to find a different way, a way that works, to check the length and contents of a list, and how to add a move to a vacant slot. Because, the first two moves are default. The third is learned. So, to summarize, I need to know how to add to a list, also checking the contents of the list to see if there is space to add. I already have it working if it is already full.
2. I coded it differently than the rest somehow, and I am completely oblivious to it.
Any Answers/Observations would be great.
Last bit of information, when I press the macro for the "Third" verb, this is the error message. "runtime error: list index out of bounds"
Koriami wrote:
Whenever you have several verbs or procs that do almost exactly the same thing with very few changes, you should re-factor them into a single proc. For example:
Keep this in mind, we'll come back to it.
Now more on your verbs. I'm not sure what you are trying to do here exactly. Let me translate one of these to English:
Here are some problems we have:
- This logic happens for every pet in the world.
- We are being overly redundant with variables
There should be no reason to have to loop through every pet in the world. If your players only have one pet, give the player a variable which points directly to that pet. If the players have 6 pets, give them a list variable with 6 elements pointing to each of the 6 pets. Now, instead of looping through ten million pets in the world, we only have to worry about 6 or however many pets a player can have. If the player can only have one pet out at a given time, give them a reference directly to the pet that is out. Now there is no searching; it's just a test if I have a pet out or not.Now on to redundancy with variables. You have 4 variables indicating which attack slot you're using. That is: usr.first, usr.second, usr.third, and usr.fourth. Then you have 4 variables indicating which attack you selected for each slot: M.Move1, M.Move2, etc. This is all completely unnecessary. Since you can only have one attack selected at a given time, you only need one variable to indicate the selection. It is irrelevant which slot is being used (first, second, third). So let us redo our pseudo-code from above:
- If I have a pet out:
- Set that pet's attack selection.
- Output a message.
Let's look at an example in code:Now look at that proc, and imagine what would change for Second() and Third(): just one number in one line! It is overly redundant and error-prone to have this logic in multiple verbs if the only thing that changes is one line; let's re-factor into a proc:
Now that we have a single proc for selecting the attack, look at what our verbs can look like:
Now, back to your code:
Next, this proc is extremely redundant. Many of the changes we've already made will have an impact. First, though, let's look at what your pseudo-code looks like:
Let's recap on some changes we've made that will help this:
So, let's try some new pseudo-code with this new setup:
To see it in code:
Stop. Hold everything. Read Crispy's "Code Red" article. Read it, understand it, bookmark it, print it off, sleep with it under your pillow, and make it breakfast in the morning. Then look up your error message in the table.
That tells you exactly why your "Third" attack isn't working. Read the error message. What does "list index" refer to? It's the index into the list; it is the number you use to access an element in the list. Where at in your "Third" verb do you use an index in a list?
What does "out of bounds" mean? It means that your index does not exist in the list. There is no element indexed at "3" in your list. In other words, M.Attacks has less than 3 attacks in it, so you can't select the third attack as it doesn't exist in M.Attacks.