NQ's dmm suite seems to be borked

ID:2107480
 
Jun 28 2016, 5:17 am (Edited on Jun 28 2016, 9:28 am)
Code:
else
    var/atom/A = new path(arglist(L))
    if (variables)
        var/nq_dmm_object/new_obj
        for (var/key in variables)
            new_obj = variables[key]
            if (istype(new_)) A.vars[key] = new_obj.Instantiate(ispath(new_obj.path, /atom/movable) ? null : A)
            else A.vars[key] = variables[key]
        return A


Problem description:
I am using NQ's dmm suite. Here, in Instantiate() hundreds of runtimes were caused due to A being null. I suspect this is because the path is not /atom (I did some testing and most were /obj). The only way I can think of around this problem is to change the else clause to:
else
    var/A = new path(arglist(L))
    if (variables)
        var/nq_dmm_object/new_obj
        for (var/key in variables)
            new_obj = variables[key]
            if (istype(new_obj)) A:vars[key] = new_obj.Instantiate(ispath(new_obj.path, /atom/movable) ? null : A)
            else A:vars[key] = variables[key]
        return A

Notice how I change var/atom/A to var/A and have to use the : operator to prevent compile errors (as opposed to the . operator). I understand the operator is widely frowned upon, since there is almost always an easy way around it. I cannot for the life of me figure out a way to not need it.
Jun 28 2016, 5:47 am
Since you're only accessing A.vars, A must be defined as a /datum or any descendant of /datum for the dot operator to compile.

What runtime errors were you getting? It shouldn't matter what type A is declared as.
Jun 28 2016, 5:50 am
1. This didn't work either, A was still null, and my debug code caught that (just a little if(!A) right after)
2.
runtime error: Cannot read null.vars
proc name: Instantiate (/nq_dmm_object/proc/Instantiate)
source file: TID,64
usr: Elena Rohtin (/mob/living/carbon/human)
src: new/obj/effect/decal/warning_s... (/nq_dmm_object)
usr.loc: the floor (167,159,3) (/turf/simulated/floor)
call stack:
new/obj/effect/decal/warning_s... (/nq_dmm_object): Instantiate(space (157,221,1) (/turf/space))
/nq_dmm (/nq_dmm): LoadMapString("\"aaa\" = (/turf/space,/area/s...", 0, 0, 0)
/nq_dmm (/nq_dmm): LoadMap('mapname.dmm', 0, 0, 0)
Mad Snail Disease (/client): Load Map('mapname.dmm', 1)
^^thats the runtime
Jun 28 2016, 6:13 am (Edited on Jun 28 2016, 6:21 am)
Update: I redid it without my debug code on the offchance I ruined something, and the source file on all of the runtimes was "You will now take people captive." line 64. I have NO idea where that came from, nor do I recognize TID from anything, come to think of it.

EDIT: TID is an id string for adding information to a feedback database (which we have disabled). At this point I'm gonna assume its just a random string from somewhere in the code.
Jun 28 2016, 8:41 am
I mean, you can just throw in an if(!isnull(A)).
Jun 28 2016, 8:45 am
What's up with the crazy spacing? Dude, your code is unreadable. Don't do that.

Also, don't use obj as a var name. That's just bad form. The obj name is close to being a keyword in DM, and is too significant to use as a var name. It'll work, sure, but it's another huge blow to readability. Even calling it just O would be ten times clearer, because when you see "obj" you expect it to be in the context of either a type path or an as clause.

As to the problem at hand: Whether you stick with the . operator or switch to the : operator will make no difference. The problem is that A is null, and you're never checking for that case. If you check to ensure that A is not null before you try manipulating its vars, the issue will go away.
Jun 28 2016, 8:47 am
In response to Lummox JR
Lummox JR wrote:
What's up with the crazy spacing? Dude, your code is unreadable. Don't do that.

That's what NQ does. Apparently it's their convention at his workplace. It was uh... a trip... trying to read his stuff.
Jun 28 2016, 9:25 am (Edited on Jun 28 2016, 9:32 am)
Yeah thats exactly how NQ wrote it, but I can delete the extra stuff here.

The problem is, A is null in exactly 511 situations every time. I added in some safety checking, but then since it doesn't give all of the extra vars, doors dont have the right orientation, pipes arent attached to each other, and things are generally screwy. The question is how do I make A not null. I have tested and path is a path and the only args are locations for all 511 objects. I tried setting it as var/A and using the : as well as the default var/atom/A and the .
Last but not least, the src file in the runtimes doesn't make any sense.
Jun 28 2016, 9:30 am
In response to Rushnut
Rushnut wrote:
Lummox JR wrote:
What's up with the crazy spacing? Dude, your code is unreadable. Don't do that.

That's what NQ does. Apparently it's their convention at his workplace. It was uh... a trip... trying to read his stuff.

Sheesh. There's no excuse for that in a professional environment, but even less to carry it anywhere else.

Jun 29 2016, 8:42 am
So I've still been trying to figure this out. Is there a way that you can do something along the lines of:
var/path/A = new(arglist(L))

instead of:
var/atom/A = new path(arglist(L))

Where path is the path I want to instantiate. I am at a total loss here, since A continues to be null despite my best efforts. In addition, I can't even tell whether or not the objects are being instantiated.
Jun 29 2016, 10:14 am
If A is null, you should check the path to make sure it's actually valid.
Jun 29 2016, 1:32 pm (Edited on Jun 29 2016, 1:51 pm)
I have. I had an if(!ispath(path)) afterwards to output a special message to the world if that was the case, but it wasn't. That is in addition to the if(!A) clause afterwards, which continues to be true for all 511 objects (unrelated to the next BYOND version, 511).

EDIT: Here's that code, for reference:
        var/atom/A = new path(arglist(L))
        if(!ispath(path))
            world << "<h2>\red ITS NOT A PATH, DUMMY</h2>"
        if(!A)
            world << "<h3><B>\blue A STILL NULL</B></h3>"
            world << ++number_of_fails
Jul 2 2016, 1:57 pm
Erm... I'm not sure why this is no longer marked as an unanswered question. I still can't figure out why A continues to be null for a very specific set of objects and none of the other thousands on the map. In every situation, path is a path and arglist(L) is just a single turf. Every time, it's the same 511 objects with a new random string elsewhere in the code as the source file.
Jul 2 2016, 2:02 pm
All questions expire after a period of inactivity.

If you can get this to happen consistently with a test case, I'd really like to see it. Variables getting messed up seems to be an issue that a couple of other games are having, and it's tied to turf loading.
Jul 3 2016, 12:04 am
This is 100% consistent. I am heading out out town for a while, but I can have one of the other coders at the server I code for send you what you need. Speaking of which, what is it that you would want to see? Unfortunately, it is a closed source server, but NQ's dmm suite is readily available. The only changes I have made were the debug code. In addition, as far as I can tell the objects ARE instantiated and even on the map, but only with the default variables of that class or any superclass.

Also, my bad on the unresolved question thing, I hadn't figured that out.
Jul 3 2016, 10:31 am
As long as you can get it down to a test case that shows the problem, that's all I need. If you can't distill it down, just send the source you're working with that shows it. I'll also need instructions on how to get to where the issue occurs and what to look for.
Jul 3 2016, 7:11 pm
Righto then. The runtimes are all exactly identical except for src, which is being instantiated, so you can see what's going in an earlier comment. I will have to contact you via PM for getting you a test example.