Adds item_worth list that says how much different atoms are worth. #13367

Kearel · 2016-06-10T22:42:32Z

Basically this is what I had previously, except without the extra baggage of all the other commits. I also decided to do the merchant machinery/programs later, as the comments/discussion mostly concentrated on this part primarily and I'd like to make sure that its done right.

Item worth is defined in the item worth module. There shouldn't be any code that is effected outside of said module (outside of the damage score). The proc get_worth() is defined to get said item's cost, and is overridden so that item_worth isn't changed.

Ccomp5950 · 2016-06-10T22:48:12Z

for the module folder, could you use an underscore for the folder name instead of a space. Names with spaces don't work that well on Linux.

Kearel · 2016-06-10T23:47:09Z

I had trouble with git mv so I decided to manually rename it.

GinjaNinja32 · 2016-06-11T20:21:31Z

this will need squashing before merge at the very least

GinjaNinja32 · 2016-06-11T20:24:46Z

code/modules/item_worth/structures.dm

+	return item_worth
+
+/obj/structure/Destroy()
+	station_damage_score += get_worth()


Is there not a more applicable proc than Destroy()? This will be called even if the structure is dismantled, not just if it's bashed to pieces.

I'm not entirely sure to be honest. I don't think the game differentiates something getting blown up and dismantled since it ultimately QDELs them.

If I can cause a zillion points of damage by repeatedly creating then dismantling diamond tables, does that not count as a bug?

I suppose I could add something to the various dismantling procs, but I don't think that it'd work very nicely, as it were, then.

I could shelve the damage score until damage/dismantling/etc becomes more generic.

GinjaNinja32 · 2016-06-11T20:30:59Z

code/modules/item_worth/items/weapon.dm

+	var/worth_multiplier = 1
+
+/obj/item/weapon/material/get_worth()
+	return material.material_worth * worth_multiplier


Are there no declarations for other /obj/item/weapon/material? This should be with them if there are.

Kearel · 2016-06-11T20:36:14Z

Commits squashed.

comma · 2016-06-17T09:35:00Z

Looks fine 👍
As a suggestion for when you implement the actual trading thing - define get_worth() at atom level, returning 0 or something, and make items and mobs override it, would avoid things like

var/worth = 0
for (var/mob/M in package)
  worth += M.get_worth()
for (var/obj/item/I in package)
  worth += I.get_worth()

since could do

for(var/atom/movable/A in package)
  worth += A.get_worth()

GinjaNinja32 · 2016-06-17T10:44:55Z

if only initial(x.parent_type) worked (and doing the following wouldn't be mega slow because BYOND), then i'd suggest doing something like

var/list/worths = list(
    /atom/movable = 0,
    /obj/foo = 5,
    /obj/structure/bar = 10
)

/atom/movable/proc/get_worth()
    var/atom/movable/t = type
    while(!(t in worths))
        t = initial(t.parent_type)
    return worths[t]

    // The above is roughly equivalent to having `worths` sorted most to least specific, and doing the following, except it should be significantly faster (not as fast as defining vars on the type though!)
    for(var/t in worths)
        if(istype(src, t))
            return worths[t]

That'd avoid potentially accidentally defining the obj types in the worth module, which we can't currently check for. Unfortunately doing initial(x.parent_type) fails because it thinks you're trying to modify parent_type, not just read it; see http://www.byond.com/forum/?post=2091430.

Really what we need is a way to define a var on a type if and only if that type already exists. I'll poke around a bit, see if I can figure out some syntax that works.

edit: Submitted a feature request over on the BYOND forums for something like:

@datum/foo // this would error at compilation if /datum/foo is not already defined
    worth = 5

@datum/bar // ditto for /datum/bar
    worth = 3

Kearel · 2016-06-17T15:26:08Z

I tried to do the parent_type thing, but yeah I ran into similar problems.

I could do something like:

item_worth = list( "/foo" = 5,
                           "/foo/bar" = 15
                          )

and then kind of parse the / and delete the end of it. I haven't been able to fiddle with the code recently though (mostly due to #13408 which I've been trying to figure out)

Techhead0 · 2016-06-18T03:05:51Z

code/modules/item_worth/items/items.dm

+	item_worth = 30
+
+/obj/item/ammo_magazine/c45m/empty
+	item_worth = 5


If get_item_worth() includes the contents thereof, I don't know why ammo magazines would be cheaper because they started empty. Furthermore, emptying a magazine doesn't change the type path, and they're all reloadable.

Its mainly for use with the merchants, since they have to spreen that stuff via initial(item_worth)

Of course I don't think any uses empty casings, so it might be a moot point.

It might be a good idea to make magazines have the same worth empty or full, then add the worth of the bullets.

it'd be a bit weird cause I don't want people buying full magazines and trading it back for profit.

I suppose I could use a secondary var but it'd again, be very odd.

I don't really understand how that would be the case, though I'm not familiar with the mechanics of buying and selling yet. The way you are doing it now is ripe for weirdness though.

For example I can buy a full mag, dump out the bullets and the mag's worth is unaffected.

Not sure what you are referring to about needing an additional var, there must be some kind of misunderstanding going on.

EDIT: missed what you said about having to use initial.

The more I think about this though the more I feel like having a /datum/economy is working against the language rather than with it. I can't think of any initialization scheme that isn't really complicated.

Datums aren't gonna work, we had this with event datums and it didn't work then either. Lists or vars, either using istype() to find it in the list or initial(parent_type) when that gets fixed.

Yeah, I tried it out, as you mentioned previously, it sends a compile error about being unable to modify the value.
So I'm unsure where to go from here.

To get the list implementation working, if slowly, replace:

var/atom/t = ispath(A) ? A : A.type while(!(t in worths)) // Find the first parent that is in the list if(t == /atom) return 0 // We hit /atom and didn't find it. t = initial(t.parent_type)

with:

var/t = null for(var/type in worths) if(istype(A, type)) t = type break if(!t) return

This will require that specific types come before base ones (ie /obj/thing must come before /obj), but it'll work until initial(t.parent_type) is fixed.

The list would be extremely large, like over 500+ entries. Would it still be viable to do it this way?

I suppose we only check this once per type, but it is still quite the loop.

Not sure. The initial(t.parent_type) should be reasonably fast, but the istype()s might be quite a bit slower.

Big lists aren't a huge problem, the problem (if there is one) is going to be all the istype() calls. Try it and see!

Kearel · 2016-06-20T01:50:27Z

Ok changed to a list. Should work now.

Kearel · 2016-06-20T02:55:02Z

Fails due to the version travis uses 509.1318, while regex needs 510.1320

GinjaNinja32 · 2016-06-20T12:32:02Z

@Kearel, change .travis.yml to say 510.1346, that'll make it pass. Have to DNM the PR for it, though, until Mloc updates us to 510 on live.

GinjaNinja32 · 2016-06-20T12:33:59Z

code/modules/item_worth/worths_list.dm

@@ -0,0 +1,740 @@
+var/list/worths = list(
+//REAGENT CONTAINERS
+					/obj/item/weapon/reagent_containers/hypospray = -90,


Should likely leave a comment on the top of the file explaining the negatives :P

GinjaNinja32 · 2016-06-20T15:52:43Z

DNM until server is running 510.

…to mercPartOne

mwerezak · 2016-07-01T06:43:55Z

I wonder if we could someday overhaul the cargo points system using this.

GinjaNinja32 · 2016-07-01T07:02:52Z

Probably :D

Kearel · 2016-07-01T18:06:50Z

When should we expect 510 to hit the server?

Kearel · 2016-07-02T02:47:27Z

Since the server's running 510 now this should be mergable, I believe.

GinjaNinja32 · 2016-07-02T11:28:21Z

Server is 510.

GinjaNinja32 · 2016-07-02T11:42:52Z

@Kearel you'll need to revert the .travis.yml changes for this to be mergeable.

…to mercPartOne

Kearel · 2016-07-02T15:16:00Z

Ok so I fixed the merge conflicts

comma · 2016-07-02T21:06:29Z

👍

Kearel · 2016-07-03T17:29:04Z

After this I think I only have to add the merchant job and a fix to figure out how to override latespawning.

GinjaNinja32 reviewed Jun 11, 2016
View reviewed changes

Kearel force-pushed the mercPartOne branch from 619cb52 to 8061b09 Compare June 11, 2016 20:30

GinjaNinja32 reviewed Jun 11, 2016
View reviewed changes

Techhead0 reviewed Jun 18, 2016
View reviewed changes

Kearel changed the title ~~Adds item_worth var to obj subtypes and mob/living~~ Adds item_worth list that says how much different atoms are worth. Jun 20, 2016

GinjaNinja32 reviewed Jun 20, 2016
View reviewed changes

GinjaNinja32 added feature labels Jun 20, 2016

Kearel mentioned this pull request Jun 20, 2016

Adds trader datums and process #13454

Merged

Squashed commit

a318ae8

Kearel force-pushed the mercPartOne branch from d189508 to a318ae8 Compare June 23, 2016 23:16

Merge branch 'dev' of https://github.com/Baystation12/Baystation12 in…

9cd4161

…to mercPartOne

Kearel force-pushed the mercPartOne branch from 04e2305 to 6a40754 Compare June 24, 2016 00:53

implements get_value proc into trader datums

5a0af63

Kearel force-pushed the mercPartOne branch from 6a40754 to 5a0af63 Compare June 24, 2016 00:57

GinjaNinja32 removed the ✋ do not merge ✋ label Jul 2, 2016

GinjaNinja32 added needs review ʘʘ labels Jul 2, 2016

Kearel added 2 commits July 2, 2016 10:03

Revert back to old travis version

c373465

Merge branch 'dev' of https://github.com/Baystation12/Baystation12 in…

5dda67b

…to mercPartOne

Fixes internal affairs rig issue

be545b1

GinjaNinja32 removed the ☠ cannot merge ☠ label Jul 2, 2016

comma merged commit acbed03 into Baystation12:dev Jul 3, 2016

Adds item_worth list that says how much different atoms are worth. #13367

Adds item_worth list that says how much different atoms are worth. #13367

Conversation

Kearel commented Jun 10, 2016

Ccomp5950 commented Jun 10, 2016 • edited

Kearel commented Jun 10, 2016

GinjaNinja32 commented Jun 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kearel commented Jun 11, 2016

comma commented Jun 17, 2016 • edited

GinjaNinja32 commented Jun 17, 2016 • edited

Kearel commented Jun 17, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mwerezak Jun 18, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kearel commented Jun 20, 2016

Kearel commented Jun 20, 2016 • edited by PsiOmegaDelta

GinjaNinja32 commented Jun 20, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GinjaNinja32 commented Jun 20, 2016

mwerezak commented Jul 1, 2016

GinjaNinja32 commented Jul 1, 2016

Kearel commented Jul 1, 2016

Kearel commented Jul 2, 2016

GinjaNinja32 commented Jul 2, 2016

GinjaNinja32 commented Jul 2, 2016

Kearel commented Jul 2, 2016

comma commented Jul 2, 2016

Kearel commented Jul 3, 2016

Ccomp5950 commented Jun 10, 2016 •

edited

comma commented Jun 17, 2016 •

edited

GinjaNinja32 commented Jun 17, 2016 •

edited

Kearel commented Jun 17, 2016 •

edited

mwerezak Jun 18, 2016 •

edited

Kearel commented Jun 20, 2016 •

edited by PsiOmegaDelta