ID:264277
 
Code:
obj/var/Material = "Bronze"
obj/Weaponry
density = 1
layer = MOB_LAYER
Kunai
name = "[Material] Kunai"
icon = 'Kunai.dmi'
Price = 500
Bump(atom/A)
var/mob/O = src.Owner
if(istype(A,/mob/))
var/mob/M = A
var/dmg = round(O.KunaiSkill + O.Taijutsu)
if(Material=="Bronze") dmg=dmg*1
if(Material=="Iron") dmg=dmg*1.5
if(Material=="Steel") dmg=dmg*2
M.Health -= dmg
view(M)<<"[M] was hit by a [src] for [dmg]"
M.Death(O)
if(istype(A,/obj/))
if(A.density)
del(src)
if(istype(A,/turf/))
if(A.density)
del(src)
verb
Drop()
for(var/obj/Weaponry/Kunai/K in usr.contents)
if(K.Amount < 1)
del(src)
else
var/drop = input("How many do you want to drop?\n- You have [src.Amount] [src]","Drop") as num|null
if(drop < 1 || drop > src.Amount) return
src.Amount -= drop
var/obj/Weaponry/Kunai/D = new(usr.loc)
D.Amount = drop
if(src.Amount == 0)
del(src)
usr.AutoSave()
Get()
set src in oview(1)
for(var/obj/Weaponry/Kunai/K in usr.contents)
Amount += 1
if(Amount < 1)
Move(usr)
else
for(var/obj/Weaponry/Kunai/X in usr.contents)
X.Amount = src.Amount
X.name = "[src] ([X.Amount])"
del(src)
usr.AutoSave()


Problem description:
Objects.dm:69:error:Material:invalid variable
Objects.dm:69:error::expected a constant expression
Which exactly is line 69 ?

You should point it out.
Mizukouken Ketsu wrote:
Objects.dm:69:error:Material:invalid variable
Objects.dm:69:error::expected a constant expression

You can't use any variables (and a lot of procs) in a prototype variable definition, it must be a constant expression. Variables can only be accessed at runtime (ie when the game, and so procs/verbs, actually run), while the object actually exists.
For this, the New() proc is used.
obj/weapon/knife
var/material
New()
src.name = "[material] knife"


On some other notes, there are some optimizations and fixes that can be made in your code:
  • In your Bump() proc:
    --you don't really need the O var. Just make sure the Owner var is initially defined as a /mob type itself.
    --It's good that you typecast, however note in cases like these you can skip it; you can eliminate the M var, then simply initially define the argument (A) as a /mob type and use it throughout the whole override.
    --When you test the same basic condition and check it for multiple values in succession, you should skip checking the rest of the values once you find a match. More literal to your code: for example, if you have confirmed that A is a mob, then there is no longer any need to check if it is also an obj or a turf - that will never happen. For this, you utilize the else if() statement - it will only execute that check if none of the previous conditions was true.
        if(ismob(Z))
    //...
    else if(isobj(Z))
    //...
    else if(isturf(Z))
    //...

    Another occurrence of the above, but a simpler one, is the comparing of the src.Material variable to multiple values in succession. It would be better to include else if there as well, however when comparing an expression to multiple values (unlike calling procs like istype()) you can use the special switch() statement, which is even more efficient.
    --In text output, you may prefer to use the \a text macro instead of actually including "a" in your text string. This causes the embedded text string after the macro to be checked, and correspondingly, the correct grammatical expression will be inserted, eg "a", "an" or "some". You can imagine this makes it look much better, as your code wouldn't produce, for example, "a Iron Kunai". :)
    --A little later down, you have needless, and repeating code:
                if(istype(A,/obj/))
    if(A.density)
    del(src)
    if(istype(A,/turf/))
    if(A.density)
    del(src)

    First, these 4 if() statements could all be condensed to a single if() statement by using the && and || operators which you are already familiar with, but I don't even see this code block as necessary. For one, there is no need to check the colliding object's density - if a Bump() already occurred, of course it is. Also, I believe you always want the object to be deleted after it hits something, no matter what the type it hits, correct? Then you can simply put del src at the bottom of the proc, not under any if statement, so it will always be executed.
  • In your Get() and Drop() verbs:
    --instead of comparing a value to 0, just use the ! operator.
    --your safety checks for players inserting bad numbers are good, but you're missing one: players can also enter decimal numbers, which shouldn't be accepted, of course. You can use the round() proc to check this.
    proc/is_whole(number)
    return number == round(number)

    In theory I think you can also use the % operator to check this, but it might not work sometimes due to BYOND's number precision.
    --the for() loops in both are pretty extraneous and weird, and worse, could lead to funky behavior. I can see what you're trying to do there however, so I'll explain how to do it.
    First, in Drop(), you seem to be implementing a safety check to see if src has a valid amount, and if not, delete it, which would also stop the proc. Then why the heck are you looping through the inventory, then handling the src var? O_o You don't need to search the inventory, you already have a reference to the object being dropped, in src, which is in the player's inventory when the verb begins. So you can simply use it instead of K, etc...
    Your Get() is also kind of a mess, which I would completely rewrite. You do need to do some object scanning there, but having an actual for() loop is unrobust. If the player accidentally has 2 or more kunais, all of their amounts will be increased, and he'll get free items! So you should break after the first iteration, or better, get rid of the loop and call locate() to grab a single kunai object.
    --lastly, instead of adding the amount after the name to display it, you may want to look into the suffix var instead. If not, you should at least use src.name instead of src when you set the name, as the latter could potentially result in a different value when being embedded.
In response to Kaioken
Not quite sure what you meant for me to do with the switch() proc or the var/mob/O = src.Owner line.

This better?
obj/Weaponry
density = 1
layer = MOB_LAYER
Kunai
icon = 'Kunai.dmi'
Price = 500
Bump(atom/A)
var/mob/O = src.Owner
if(ismob(A))
var/mob/M = A
var/dmg = round(O.KunaiSkill + O.Taijutsu)
if(Material=="Bronze") dmg=dmg*1
if(Material=="Iron") dmg=dmg*1.5
if(Material=="Steel") dmg=dmg*2
M.Health -= dmg
view(M)<<"[M] was hit by \a [src] for [dmg]"
M.Death(O)
del(src)
New()
src.name = "[src.Material] Kunai"
..()
verb
Drop()
for(var/obj/Weaponry/Kunai/K in usr.contents)
if(!src.Amount)
del(src)
else
var/drop = input("How many do you want to drop?\n- You have [src.Amount] [src]","Drop") as num|null
if(!drop || drop > src.Amount) return
src.Amount -= drop
var/obj/Weaponry/Kunai/D = new(usr.loc)
D.Amount = drop
if(!src.Amount)
del(src)
usr.AutoSave()
Get()
set src in oview(1)
for(var/obj/Weaponry/Kunai/K in usr.contents)
Amount += 1
break
if(!Amount)
Move(usr)
else
for(var/obj/Weaponry/Kunai/X in usr.contents)
X.Amount = src.Amount
X.suffix = "([X.Amount])"
del(src)
usr.AutoSave()
In response to Mizukouken Ketsu
Mizukouken Ketsu wrote:
Not quite sure what you meant for me to do with the switch() proc or the var/mob/O = src.Owner line.

1)
Steps of Code Evolution:
if(MyVar == "value1")
//...
if(MyVar == 2)
//...
if(MyVar == X)
//...

This is pretty bad; even if it is confirmed that MyVar equals "value1", the proc will continue to pointlessly check if it also equals 2, or X, but we already know it basically wouldn't, since it equals "value1" instead.
So, we use the else-if statement, which means, if none of the previous if()s were true, check this condition and if it's true, execute this statement. See the if() statement's Ref entry for more info.
if(MyVar == "value1")
//...
else if(MyVar == 2) //if it didn't equal "value1", check 2
//...
else if(MyVar == X) //if it didn't equal "value1" or 2, check X
//...

For general conditions (like you had with calling a proc, ie istype()), this is basically as efficient as you get, but for comparing (the == operator) an expression to multiple values, there's a special statement which does it efficiently, like the above, but even better - basically put. That's the switch() statement, look it up.
switch(MyVar)
if("value1")
//...
if(2)
//...
if(X)
//...


2) Instead of declaring a new variable O and setting it to the value of src.Owner, you can just use the src.Owner variable in its place, as long as you have properly defined Owner as /mob typed in its declaration.

This better?

Yes, but there are still considerable parts in my previous post you haven't accounted for yet, so we'll see if there's still something left until you do 'em. =P
In response to Kaioken
Ah, I get it. I chose to use the switch statement :D I also added the proc to round off whatever number they input for the drop variable. Moved the Material obj/var definition to under the obj definition. There's a small issue described at the bottum.

proc/Num2Whole(number)
return number == round(number)

obj/Weaponry
density = 1
layer = MOB_LAYER
Knife
icon = 'Knife.dmi'
Price = 500
var/Material
Bump(atom/A)
var/mob/O = src.Owner
if(ismob(A))
var/mob/M = A
var/dmg = round(O.KunaiSkill + O.Taijutsu)
switch(Material)
if("Bronze") dmg=dmg*1
if("Iron") dmg=dmg*1.5
if("Steel") dmg=dmg*2
M.Health -= dmg
view(M)<<"[M] was hit by \a [src] for [dmg]"
M.Death(O)
del(src)
New()
src.name = "[src.Material] Knife"
..()
verb
Drop()
for(var/obj/Weaponry/Knife/K in usr.contents)
if(!src.Amount)
del(src)
else
var/drop = input("How many do you want to drop?\n- You have [src.Amount] [src]","Drop") as num|null
if(!drop || drop > src.Amount) return
src.Amount -= Num2Whole(drop)
var/obj/Weaponry/Knife/D = new(usr.loc)
D.Amount = drop
if(!src.Amount)
del(src)
usr.AutoSave()
Get()
set src in oview(1)
for(var/obj/Weaponry/Knife/K in usr.contents)
Amount += 1
break
if(!Amount)
Move(usr)
else
for(var/obj/Weaponry/Knife/X in usr.contents)
X.Amount = src.Amount
X.suffix = "([X.Amount])"
del(src)
usr.AutoSave()


As a side note, the reason I wanted the Knife's name to be "[material] Knife" was because I have a Blacksmithing profession in the game, and the players can blacksmith Bronze, Iron, or Steel knives (you might've been able to guess from the switch() statement ;p). Okay well on to the issue. I used the below code to blacksmith an Iron knife, yet when the obj is added to my inventory, the name is "Knife". Why didn't it obtain the Material text to put in front of the name? I thought the above coding's New() proc covered that.

mob
Iron/verb
IKnife()
if(src.Smithing) return
for(var/obj/Bars/Iron_Bar/I in src.contents)
if(!I)
src<<"You don't have enough Bronze bars!"
return
del(I)
break
winset(src,"Blacksmithing2","is-visible=false")
winset(src,"Iron","is-visible=false")
src.Smithing = 1
src.Smithing()
var/obj/Weaponry/Knife/K = new(src)
K.Material = "Iron"
src.verbs -= typesof("/mob/Iron/verb")
src.verbs -= typesof("/mob/Blacksmithing/verb")
src.Smithing = 0
In response to Mizukouken Ketsu
The proc I posted doesn't convert a decimal number to a whole one. >_> Read that part in my main post again. Also look up round() please. =P

For the name problem,
>           var/obj/Weaponry/Knife/K = new(src)
> K.Material = "Iron"


The problem is that you're only setting the Material AFTER the New() proc runs... (new() calls New()).
You shouldn't initialize objects like this most of the time anyway; do it properly in the object definition tree, which will also ensure the value is set internally by DM, before New() is even ran.
obj/weapon
var/material
knife
New()
name = "[src.material] Knife"
//for this code, could add a capitalization of the\
first character here with copytext() and uppertext()

average_knife
icon_state = "iron knife"//...
material = "iron"
badass_knife
icon_state = "steel knife"
material = "steel"
rare_knife
material = "platinum"
//you get the picture
In response to Kaioken
Okay great. Next step! :D Right now whenever I blacksmith some Knives, it creates a whole new group of 3 knives in my inventory. How do I make it so it adds to the existing /obj/Weaponry/Knife/Iron pile IF it exists (of course if it doesn't exist in the player's contents, make the new set).

obj/Weaponry
density = 1
layer = MOB_LAYER
Kunai
Kunai_Exploding
Shuriken
Shuriken_Windmill
Needle
Knife
icon = 'Knife.dmi'
Price = 500
var/Material
Bump(atom/A)
var/mob/O = src.Owner
if(ismob(A))
var/mob/M = A
var/dmg = round(O.KunaiSkill + O.Taijutsu)
switch(Material)
if("Bronze") dmg=dmg*1
if("Iron") dmg=dmg*1.5
if("Steel") dmg=dmg*2
M.Health -= dmg
view(M)<<"[M] was hit by \a [src] for [dmg]"
M.Death(O)
del(src)
New()
src.name = "[src.Material] Knife"
src.suffix = "([Amount])"
..()
Bronze
Material = "Bronze"
Amount = 3
Iron
Material = "Iron"
Amount = 3
Steel
Material = "Steel"
Amount = 3
verb
Drop()
for(var/obj/Weaponry/Knife/K in usr.contents)
if(!src.Amount)
del(src)
else
var/drop = input("How many do you want to drop?\n- You have [src.Amount] [src]","Drop") as num|null
if(!drop || drop > src.Amount) return
src.Amount -= Num2Whole(drop)
var/obj/Weaponry/Knife/D = new(usr.loc)
D.Amount = drop
if(!src.Amount)
del(src)
usr.AutoSave()
Get()
set src in oview(1)
for(var/obj/Weaponry/Knife/K in usr.contents)
Amount += 1
break
if(!Amount)
Move(usr)
else
for(var/obj/Weaponry/Knife/X in usr.contents)
X.Amount = src.Amount
X.suffix = "([X.Amount])"
del(src)
usr.AutoSave()

mob
Iron/verb
IKnife()
if(src.Smithing) return
for(var/obj/Bars/Iron_Bar/I in src.contents)
if(!I)
src<<"You don't have enough Bronze bars!"
return
del(I)
break
winset(src,"Blacksmithing2","is-visible=false")
winset(src,"Iron","is-visible=false")
src.Smithing = 1
src.Smithing()
new/obj/Weaponry/Knife/Iron(src)
src.verbs -= typesof("/mob/Iron/verb")
src.verbs -= typesof("/mob/Blacksmithing/verb")
src.Smithing = 0
In response to Mizukouken Ketsu
ALSO, if you wouldn't mind correcting the first for() loop for me? The if(!I) line is supposed to mean "if I doesn't exist in src.contents", yet when I test it, meaning I don't have the obj defined in the for()'s args, it doesn't display that message and return. It skips over that part. What do I need to do to make it check to see if it exists in src.contents?
In response to Mizukouken Ketsu
There are still some parts you haven't fixed, like your Get() and Drop() verbs.
And you're still using "Num2Whole" wrong, that proc I included only returns 1 or 0, not the number rounded off.

Mizukouken Ketsu wrote:
The if(!I) line is supposed to mean "if I doesn't exist in src.contents", [...]

Yet it doesn't make sense; by writing for(var/obj/.../I in src.contents), you're essentially saying "execute the code below this once for every obj that is in the contents list". So checking if I exists/isn't null, or if I is inside the contents (which you'd use the in operator to check BTW), doesn't make sense; it will always be when that code runs - when there's no such objects in the list used with the for()-list construct, or the list is invalid/null, the code under the for() simply doesn't run at all.
Also, as I've previously said, if all you need to do is check or get one object of a specific type of a list, use the locate() proc (look it up), since a for() loop is not needed for that. =P
In response to Kaioken
Would I use that?
mob
Smelting/verb
Bronze()
set hidden = 1
if(src.Smelting) return
src.Smelting = 1
winset(src,"Blacksmithing","is-visible=false")
var/obj/Ores/Copper_Ore/A = /obj/Ores/Copper_Ore
var/obj/Ores/Tin_Ore/B = /obj/Ores/Tin_Ore
if(A in src.contents && B in src.contents)
del(A); del(B)
if(!A in src.contents || !B in src.contents)
src<<"You don't have enough [A.name] and/or [B.name]"
return
src.Smelting()
new/obj/Bars/Bronze_Bar(src)
src.verbs -= typesof("/mob/Smelting/verb")
src.Smelting = 0
In response to Mizukouken Ketsu
Mizukouken Ketsu wrote:
Would I use that?
>           var/obj/Ores/Copper_Ore/A = /obj/Ores/Copper_Ore
> var/obj/Ores/Tin_Ore/B = /obj/Ores/Tin_Ore

Note that even though you're typecasting the A and B vars as object, that doesn't really mean anything and does not make them objects - you are setting them to type paths, not instantiated objects.
>           if(A in src.contents && B in src.contents)
> del(A); del(B)

So this is a no go; if anything, contents contains object instances, but you're checking if there are type paths in it, and that's strictly never the case; you're also then attempting to delete those type paths, but they're not a valid value for deletion, only object instances (again) are.
>           if(!(A in src.contents) || !(B in src.contents))
> src<<"You don't have enough [A.name] and/or [B.name]"
> return
>

First, note to get statements like these to work as intended you need to add the extra parentheses as above, because 'in' has a lower precedence than '!'. But if you look through this again, you'll probably see it doesn't make sense; even if up to here the code was proper, you've supposedly deleted the objects in contents - then you're immediately checking for more objects in it. What you should really be using is an 'else' statement - pretty much never go for checking conditions like these:
if(MyExpression)
//Do_stuff
if(!MyExpression) //use 'else' here instead!
//Do_something_else


Bear with me here; I mentioned to use locate(). Look these things up! You want the the first version/syntax, it's what allows you to find an object of a specific type in a list.
In response to Kaioken
Do I use locate() in the variable definitions, or like in the second code snippet?

            var/obj/Ores/Copper_Ore/A = locate(src.contents)
var/obj/Ores/Tin_Ore/B = locate(src.contents)
if(A in src.contents && B in src.contents)
del(A); del(B)
else if(!(A in src.contents) || !(B in src.contents))
src<<"You don't have enough [A.name] and/or [B.name]"
return

...
            if(A.loc == src.contents && B.loc == src.contents)


EDIT
Okay the first snippet works for the else if(), but when I do have the object in my contents, it still says I don't have enough x.x

mob
Smelting/verb
Bronze()
set hidden = 1
if(src.Smelting) return
winset(src,"Blacksmithing","is-visible=false")
var/obj/Ores/Copper_Ore/A = locate(src.contents)
var/obj/Ores/Tin_Ore/B = locate(src.contents)
if(A in src.contents && B in src.contents)
del(A); del(B)
else if(!(A in src.contents) || !(B in src.contents))
src<<"You don't have enough copper ore and/or tin ore"
return
src.Smelting = 1
src.Smelting()
new/obj/Bars/Bronze_Bar(src)
src.verbs -= typesof("/mob/Smelting/verb")
src.Smelting = 0
In response to Mizukouken Ketsu
Mizukouken Ketsu wrote:
Do I use locate() in the variable definitions, or like in the second code snippet?

Both your uses are wrong. You can use locate() wherever you prefer at the time, variable declaration or not, but you've used a nonexistent syntax.

Kaioken wrote:
Bear with me here; I mentioned to use locate(). Look these things up! You want the the first version/syntax, it's what allows you to find an object of a specific type in a list.


As for the 2nd code snippet, loc IS NOT locate(). Do not confuse. They are not so tied to each other at all. loc is a variable on an atom containing its location (which is another atom, or null). locate() is a proc used to find an object (including an atom) by supplying a characteristic of that object (depending on the locate() syntax used), which is uses to find the right one. They are separate.

It feels as if you've skipped reading the reference entry, because it explains it quite clearly with no much room for mistakes.
Format: locate(Type) in Container
...
Arguments:
Type: An object prototype or tag. <small>If locate() is being used in an assignment to a variable with a declared type, this argument is optional and will default to the type of the variable being assigned. </small>
Container: An optional container object. (The default is world.)
Returns:
An object of the specified type [from the specified list]

Type is a type path. Container can be a list, or an /atom (or /world) object - in which case its contents will be used.
You can figure out the rest. =P
In response to Kaioken
Man my blondness has REALLY been showing hasn't it? I put the in src.contents within the locate() proc xD I feel so stupid right now v.v;


Anyways. ere's the finalized coding. It works 100%! :D Thanks Kaioken! ^-^ Now can you address the issue in [link]
mob
Smelting/verb
Bronze()
set hidden = 1
if(src.Smelting) return
winset(src,"Blacksmithing","is-visible=false")
var/A = locate(/obj/Ores/Copper_Ore) in src.contents
var/B = locate(/obj/Ores/Tin_Ore) in src.contents
if(!(A in src.contents) || !(B in src.contents))
src<<"You don't have enough copper ore and/or tin ore"
return
del(A); del(B)
src.Smelting = 1
src.Smelting()
new/obj/Bars/Bronze_Bar(src)
src.verbs -= typesof("/mob/Smelting/verb")
src.Smelting = 0
In response to Mizukouken Ketsu
Mizukouken Ketsu wrote:
Man my blondness has REALLY been showing hasn't it? I put the in src.contents within the locate() proc xD I feel so stupid right now v.v;

It's okay, everyone makes mistake. Actually, that's great that you just slipped, I thought you didn't or perhaps wouldn't understand it. >_> =P

Anyways. ere's the finalized coding. It works 100%! :D Thanks Kaioken! ^-^ Now can you address the issue in [link]

What, you have more?
Just joking. XD It is also most convenient, as I've addressed it already in the subsequent [link]; see the last sentence. You need to use locate() again. Since that version of locate() finds an object in a specific container, you can naturally use it to check if it doesn't find one, too, in which case it will return false. If it returned true, then you can use that object, of course.
In response to Kaioken
*facewall* I should've noticed that... I'll post back x.x
In response to Kaioken
I'm positive there's a simple answer to this, but I can't seem to realize why. (noted in the coding)

    Bronze/verb
BKnife()
if(src.Smithing) return
var/A = locate(/obj/Bars/Bronze_Bar) in src.contents
if(!(A in src.contents))
src<<"You don't have enough bronze bars"
return
del(A)
winset(src,"Blacksmithing2","is-visible=false")
winset(src,"Bronze","is-visible=false")
src.Smithing = 1
src.Smithing()
var/X = locate(/obj/Weaponry/Knife/Iron) in src.contents
if(!(X in src.contents))
new/obj/Weaponry/Knife/Bronze(src)
else
X.Amount += 5 //X.Amount is undefined variable
X.suffix = "[X.Amount]" //X.suffix and X.Amount are undefined variables
src.verbs -= typesof("/mob/Bronze/verb")
src.verbs -= typesof("/mob/Blacksmithing/verb")
src.Smithing = 0
In response to Mizukouken Ketsu
Mizukouken Ketsu wrote:
I'm positive there's a simple answer to this, but I can't seem to realize why.

Fortunately, you are correct.
Observe:
var/X = locate(/obj/Weaponry/Knife/Iron) in src.contents
X.Amount += 5 //X.Amount is undefined variable

When you try to access an object's var or proc by the '.' operator and a variable whose type is not defined, you'll always get an error. In this case, you'll (probably) naturally also get an error if you define the var, as, say, /obj. If it might not be clear what I'm referring to by defining the type of the var, it is the type you supply in the variable declaration, and is used for such error-checking (which just produced an error for you). The DM Reference on 'var' says: var/Type/Name - you know already how to define a var's type.
The full variable declaration line would then be:
var/obj/Weaponry/Knife/Iron/X = locate(/obj/Weaponry/Knife/Iron) in src.contents

As you can see, this is kind of annoying and long; this is why the locate() proc, just like the new() proc, offers a shorthand option for when you use a variable that has a declared type:
var/obj/Weaponry/Knife/Iron/X = locate() in src.contents

In this case the Type is not supplied - the variable used (X)'s type will be automatically used instead.
In response to Kaioken
Oh thank god! lol That much I did not know, nor see in the guide. Thanks yet AGAIN Kaioken! x.x
Page: 1 2