ID:2024969
 
Not a bug
BYOND Version:509
Operating System:Linux
Web Browser:Chrome 48.0.2564.82
Applies to:Dream Daemon
Status: Not a bug

This is not a bug. It may be an incorrect use of syntax or a limitation in the software. For further discussion on the matter, please consult the BYOND forums.
Tested in 509.1319, but has been an issue for a while, I just never got around to looking into it.

Descriptive Problem Summary:
I have no idea what's going on here, just that something is. My attempts to make this into a nice small demo have failed, so I'm going to have to do this as a download-the-BS12-source example :(

Numbered Steps to Reproduce Problem:
1. Download the Baystation12 source tree; zip file here
2. Insert "/datum/New() return ..()" anywhere before line 12 of code/world.dm,
var/global/datum/global_init/init = new ()

3. Compile, run, and watch the entire codebase runtime.

A useful test position is line 80 of code/_helpers/global_lists.dm, which is where the first runtime occurs (the line "hair_styles_list[H.name] = H"). At this point, my testing has found:

- hair_styles_list is not a list, datum, or any other built-in type
- hair_styles_list is not null, but behaves very much like null (hair_styles_list.type runtimes "Cannot read .type", call(hair_styles_list, "something")() fails "Cannot execute null.something()")
- hair_styles_list is boolean true
- "[hair_styles_list]" is the empty string

Code Snippet (if applicable) to Reproduce Problem:
(This is an example of what the top of code/world.dm could look like following the above instructions)
/datum/New()
return ..()
/*
[...]
*/

var/global/datum/global_init/init = new ()

// [...]


Expected Results: The insertion of "/datum/New() return ..()" in any file would not affect the execution results of any code.

Actual Results: It broke the entire codebase.

Does the problem occur:
Every time? Or how often? Every time.
In other games? No.
In other user accounts? Unknown.
On other computers? Unknown.

When does the problem NOT occur? N/A

Did the problem NOT occur in any earlier versions? If so, what was the last version that worked? Unknown.

Workarounds: None

You didn't list what the runtime errors were, which is kind of important.

Also, you should never specify a return value in New(). Does simply calling ..() without the return statement have the same effect?
It does, yes. Putting identical code (including return value) below instead of above the definition of init doesn't break it, though.

Runtime errors:
runtime error: bad index
proc name: makeDatumRefLists (/proc/makeDatumRefLists)
source file: global_lists.dm,80
usr: (src)
src: null
call stack:
makeDatumRefLists()
/datum/global_init (/datum/global_init): New()
world: ()


runtime error: bad index
proc name: New (/datum/configuration/New)
source file: configuration.dm,231
usr: null
src: /datum/configuration (/datum/configuration)
call stack:
/datum/configuration (/datum/configuration): New()
load configuration()
/datum/global_init (/datum/global_init): New()
world: ()


runtime error: Cannot execute null.raise event().
proc name: Destroy (/datum/Destroy)
source file: destroyed.dm,7
usr: null
src: /datum/global_init (/datum/global_init)
call stack:
/datum/global_init (/datum/global_init): Destroy()
qdel(/datum/global_init (/datum/global_init))
/datum/global_init (/datum/global_init): New()
world: ()


runtime error: bad index
proc name: IsPooled (/proc/IsPooled)
source file: datum_pool.dm,79
usr: (src)
src: null
call stack:
IsPooled(/datum/global_init (/datum/global_init))
/datum/global_init (/datum/global_init): finalize qdel()
qdel(/datum/global_init (/datum/global_init))
/datum/global_init (/datum/global_init): New()
world: ()


runtime error: list index out of bounds
proc name: New (/obj/structure/sign/poster/New)
source file: contraband.dm,95
usr: null
src: the poster (/obj/structure/sign/poster)
src.loc: the plating (132,112,1) (/turf/simulated/floor/plating)
call stack:
the poster (/obj/structure/sign/poster): New(the plating (132,112,1) (/turf/simulated/floor/plating), null, 1)


runtime error: Cannot read null.language
proc name: set species (/mob/living/carbon/human/proc/set_species)
source file: human.dm,1113
usr: (src)
src: the unknown (/mob/living/carbon/human/monkey/punpun)
src.loc: the linoleum (135,141,1) (/turf/simulated/floor/lino)
call stack:
the unknown (/mob/living/carbon/human/monkey/punpun): set species("Human", 1)
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino), "Monkey")
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino))
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino))


runtime error: Division by zero
proc name: SetUIValueRange (/datum/dna/proc/SetUIValueRange)
source file: dna2.dm,178
usr: the unknown (/mob/living/carbon/human/monkey/punpun)
src: /datum/dna (/datum/dna)
usr.loc: the linoleum (135,141,1) (/turf/simulated/floor/lino)
call stack:
/datum/dna (/datum/dna): SetUIValueRange(16, 1, 0, 1)
/datum/dna (/datum/dna): ResetUIFrom(the unknown (/mob/living/carbon/human/monkey/punpun))
/datum/dna (/datum/dna): ready dna(the unknown (/mob/living/carbon/human/monkey/punpun))
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino), "Monkey")
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino))
the unknown (/mob/living/carbon/human/monkey/punpun): New(the linoleum (135,141,1) (/turf/simulated/floor/lino))


None of these happen without the "/datum/New() ..()" before the definition of init.
Bump. This just caused our CI to fail a build because one of our developers #included files from a map definition, which gets loaded at the beginning of the dme instead of the end if the map is being overridden.

Still an issue in 510.1345.
Poked around a bit at this, finally managed to get a test case setup.

#define DEBUG

// Comment this definition to un-break everything
/datum/New()
..()

var/global/datum/init = new ()
var/global/list/wat = list()

/datum/New()
world.log << "Truth value: [!!wat]"
world.log << "String value: '[wat]'"
world.log << "Is-list: [istype(wat, /list)]"
world.log << "Is-null: [isnull(wat)]"
world.log << "Length: [wat.len]"
..()


Expected output:
Truth value: 1
String value: '/list'
Is-list: 1
Is-null: 0
Length: 0


Actual output:
Truth value: 1
String value: ''
Is-list: 0
Is-null: 0
runtime error: Cannot read .len
proc name: New (/datum/New)
source file: init.dme,15
usr: null
src: /datum (/datum)
call stack:
/datum (/datum): New()
world: ()
Lummox JR resolved issue (Not a bug)
wat doesn't exist yet because its currently processing init. if you abuse define time assignment of reference types, oddities with linear code processes is kinda the downfall.

the "unbroken" behavior should be the proper one technically. (at least, in your test case)

The fact it works if you comment out the first definition is bloody fucking odd.

that means that list() is treated specially, and i'm not shocked that it is, or that that new() is for some reason running after list()

either way there be dragons in these hills.

any comment lummox?
Your test case demonstrates that this is a non-bug.

Look at the order in which you're doing things. You have two versions of datum/New() in your code, which doesn't guarantee they'll run in any particular order. When you create the global init datum, you haven't created the wat list yet. Because the datum has a New() proc, that proc is immediately executed within the hidden world/_init(), and at that point wat is still null. The wat list isn't initialized until control returns to world/_init().

[edit]
MSO got to it quicker than me, and more eloquently.
so why does it work if you comment out the first New(), list() should not have ran
If that's the case, Lummox, why does removing the first datum/New() stop it runtiming? If it was just a case of initialisation order, the number of datum/New() definitions wouldn't matter, only the presence of them.
In response to GinjaNinja32
GinjaNinja32 wrote:
If that's the case, Lummox, why does removing the first datum/New() stop it runtiming? If it was just a case of initialisation order, the number of datum/New() definitions wouldn't matter, only the presence of them.

Like MSO said, it's weird. If I dig into the compiler I may or may not be able to get you an answer. It may be something as simple as the proc definition order being significant to the world startup. Defining datum/New() at the top may be forcing the datum initialization to take on a different order in the init proc. DM actually includes no guarantees as to the order in which compile-time var inits are done in the hidden init proc.
This might be something to still look into changing to ensure list() and other snowflaked () procs like icon() and whatnot shit happens first. It seems if the type doesn't exist yet it adds it to a separate array that gets init'd after types that do like list()


The reason I say this is because I'm 99% sure alot of games unknowingly took advantage of this quirk and the last thing you want is for some compiler refactor down the line to cause a buttfuck ton of bug reports over a quirk.
I would be surprised if too many games took advantage of a particular init-proc order. GingaNinja has become the patron saint of finding weird compiler errors, and I think I'd have heard more about something like this long before now.

I'm sure there's a reason for the init order being different, which would probably show up if I took a lot of time to look for it. But boy is that not high on my list, because running through the compiler in these situations is a nasty business.
In response to Lummox JR
I would be surprised if too many games took advantage of a particular init-proc order.

The reason this got found was that Bay uses it; evidently, at least one game uses it, and I'd be surprised if we were the only ones :P

GingaNinja has become the patron saint of finding weird compiler errors

I like poking things and seeing how they break. Sometimes those things are BYOND.

I'm sure there's a reason for the init order being different, which would probably show up if I took a lot of time to look for it. But boy is that not high on my list, because running through the compiler in these situations is a nasty business.

I've already PRed a workaround/fix/whatever to Bay so we don't run into this, but I'm a little worried about your earlier post;

DM actually includes no guarantees as to the order in which compile-time var inits are done in the hidden init proc.

Does this mean that, even if the vars were defined list then datum, there's a chance a future change could make the datum initialise first again, potentially without warning?
In response to GinjaNinja32
GinjaNinja32 wrote:
Does this mean that, even if the vars were defined list then datum, there's a chance a future change could make the datum initialise first again, potentially without warning?

It's really unlikely, but then it's not likely that the current behavior you're seeing (where defining a proc changes the order) would happen either. The compiler tends to do things very sequentially.

Truth is I think it's worth looking into the behavior change you're seeing at some point, just not now. I don't like that the order is turning out to be a little bit capricious.