[READY] NSFW Gun Update (I hope it's ready anyway, no flaws with the code itself)

[king5327]

Fix: NSFW batteries can be charged again
Add: The NSFW can now be put into rechargers to charge the batteries directly. All three batteries charge at once in this configuration.
Add: NSFW batteries can now be removed one by one from the magazine (FIFO)
Change: NSFW batteries are now a subclass of ammo_casing/rechargeable instead of just ammo_casing. In case somebody else wants a different magazine gun with energy shots.

[deathride58]

Changes are gonna have to be modularized or sent upstream

[king5327]

Doesn't Git have a merge feature exactly so that isn't a problem?

[deathride58]

Thing is, we're using a mirror bot that's unable to handle merge conflicts on its own whatsoever. On top of that, expecting maintainers to remember every single unique thing that's changed from upstream is simply unreasonable. Modularizing your code will ensure that your changes aren't completely overwritten by upstream syncs or mirrored PRs from the mirror bot, and also has the added benefit of making it much easier to see what's changed from the base code.

[king5327]

I'm not going to split code for the same object over multiple files, and IIRC having two procs with identical signatures causes undefined behaviour. I'm not going to rely on the second one to be parsed being the one that executes until I see it defined absolutely in the documentation.

[king5327]

Don't merge, one minor bug.

[deathride58]

Just because it isn't documented at all doesn't mean it doesn't work. If that were the case, shit like spawn(-1) being an effective way to make a proc asynchronous simply wouldn't be a thing. DM is a language that has a LOT of undocumented behavior, from shit as simple as hex/octal notation (0x for hex, 0 for octal), to mostly useless shit like DUNG_VERSION (returns the exact DM version of the compiler), to insanely useful shit like initial(PATHSTRING:VAR) (returns the value of a var of a type defined in a type string without having to go through the trouble of actually creating an instance of that type), and to the infuriating shit like trying to access a var exclusive to a type defined in a var without first running an istype() check on that var (causes a runtime unless the : operator is used to check that var).

The way DM works, duplicate proc definitions will execute their code in the order the files are defined in the DME, though failing to include a . = ..() in the duplicate will result in the proc being completely overwritten during compiling. If you're familiar with pretty much any other object-oriented programming language, this is the exact same kind of behavior those languages follow when a function is defined with the same exact name as another. Since ..() (very lazily documented here) happens to call the proc it's overriding, this means that modularity is very, very easy with BYOND, to the point where there's very little excuse to do so in a downstream of a project as expansive as SS13.

To put things into an example, let's say we have a file named "AAA.dm" with the contents of...

/world
    var/peepbeep = 5

/world/proc/beep()
    return peepbeep*20

/world/New()
    . = ..()
    world >> "there's a total of [beep()] beeps here!"

If this file is compiled on it's own in DM and run, you'll end up with a dreamseeker window with nothing but "there's a total of 100 beeps here!" in the chatbox.

Now let's add another file named "ZZZ.dm" with the contents of...

/world
    peepbeep = 50

/world/beep()
    . = ..()
    return . + 50

Aaaaand compile and run. You'll end up with "there's a total of 1050 beeps here!" in the chatbox, showing that the beep() proc and peepbeep var were both successfully overridden. Toy around with it on your own, and you'll see for yourself how easy it is to modularize code.

Also protip: Everything in the modular_citadel folder will always load after the code folder. All you really need to do is make a file with the same relative directory in the modular_citadel folder as the file you're overriding stuff from, and the rest should be relatively easy to pick up.

[deathride58]

Also, the path change of the NSFW battery really isn't necessary at all. All it'll do is break compatibility with upstream changes that touch the NSFW and make every single mirrored PR touching the NSFW end up with merge conflicts. Since the base /nsfw_batt typepath doesn't have any mechanical differences whatsoever from the rechargeable typepath, future contributors wanting to make a rechargeable clip can do so by making a subtype of the /nsfw_batt typepath.

[king5327]

In that case, I'll make a PR upstream to make the nsfw_batt subclass of ammo_casing/rechargeable. I do not want people making rechargeable ammo based off the cryptically named ammo_casing/nsfw_batt. However, unless one of them notice this PR, I'm going to keep everything else here.

[king5327]

Response to your big textbox: what would happen if Virgo were a modular codebase to, say, Bay instead of its own complete branch, and you tried to override a proc that was already overriden elsewhere by Virgo. Ending up with:

/object/proc/foo()
   doonething

/object/foo
    doanotherthing

/object/foo
    doathirdthing

All of which are in separate files. What if they had behaviours that conflicted but were both necessary, in such a way that fallthrough with ..() would not occur even though ..() would also provide an interpretation for that same action.

/object/foo
    if(samex)
        doanotherthing
    else ..()

/object/foo
    if(samex)
        doathirdthing
    else ..()

In this case, doanotherthing and doathirdthing would be arbitrarily selected based on whichever one got called first. Knowing DM, it would probably run doathirdthing consistently if it were the last one to be parsed at compiletime, however that does not mean relying on that behaviour is a good idea. Again, if it's not documented, it's undocumented behaviour. Will it work? Yes. Is it guaranteed to work next byond update? No.

I would much rather have a keyword for this sort of thing. I do NOT want to define two different procs with the same signature. I'll be posting more about it on the BYOND forums.

[jakeramsay007]

You're not defining the proc with the same signature, you're overwriting it while keeping the parent proc intact. That's what .=..() does

[king5327]

So, you've got:
/obj/thing/proc/function <- We'll call this choice A.
/obj/thing/function <- We'll call this choice B.

How do I call choice A without calling B?
What happens if I add a choice C? /obj/thing/function

Let's say I have preexisting code:

var/obj/thing/thinga = new /obj/thing
var/quack = thinga.function()

If adding choice B to the codebase makes thinga.function call choice B instead of choice A, then choice B has taken over the signature, and choice A is basically an anonymous function addressable only by ..() in choice B.

[deathride58]

Still needs to be modularized. No amount of bikeshedding or arguing code semantics is going to give you a free pass to break upstream compatibility for things that can easily be modularized.

[nik707]

Closing due to inactivity, reopen if fixed.