Closed [PATCH] Typos in dungeon brush definitions

There are several places where a dungeon brush is defined like this:

"brush" : [ [ "clear" ], [ "object", "birdgroundlantern", { "direction" : "left" }, { "parameters" : { "defaultLightState" : false } } ] ]​

Note that "parameters" is a separate dict as the fourth argument. However most uses of "parameters" are embedded in the third argument, e.g.:

"brush" : [ [ "clear" ], [ "object", "crystaltorch", { "parameters" : { "defaultLightState" : false } } ] ]​

Since there are only a few instances of the first pattern, and every single one of them is for a birdgroundlantern, I assume this is a mistake that propagated by copy/paste.

Patch against current stable that fixes every instance of this I could find: https://gist.github.com/eevee/07288e27f2f191ce2449#file-starbound-brush-parameters-patch

 

I'll be honest, I don't see a bug here. Lists are unordered and you should not write code expecting to find the same entry in the same place in each list (that's what dicts can be used for). Note that your second example line does not have the extra dict as an entry anywhere in the list as like the first one does.

I don't see any of the keys or values misspelled, so no typos either.

(If you're reporting this because you have code that's choking on processing these lines, you need to fix your code. These entries look RFC compliant to me, although I haven't done extensive checking with them against the RFC spec.)

 

It's perfectly good JSON. The problem is that the other 8,033 object brushes use a three-element list: the string "object", the name of the object, and a dict of arbitrary named arguments. (I say "arbitrary", but in practice the only names are "direction" and "parameters".)

These 10 definitions, all for the same object, are the only ones that use four elements. There are 745 other cases where both "direction" and "parameters" are given, and all of them put both in the same dict.

Thus I'm guessing that this is a mistake that was copy/pasted around, and the engine happens to be ignoring the fourth element here, because it doesn't expect anything to be there. Nothing is actually broken in the game, but there's data intended to be used that's being ignored.

(btw, lists are most certainly ordered — ordering is the one thing they're for )

 

(I meant they may not be in the same order between objects, not that they're unordered within the same list. Item 1 in list A could be item 4 in list B and be valid depending on the use case, although it would be better to preserve the same order between the same types of objects. I certainly did word it wrongly though. )

Are you sure it's being ignored? Have you tried creating your own object with different parameters and seeing how that affects the object in-game? The game does use defaults for some parameters, so it's possible that these few objects with an extra parameter are just specified so they override the default(s).

You're going to have a hard time convincing someone if you can't demonstrate that what you're referring to is indeed broken in some manner. I would suggest setting up a simple mod with a test item so you can demonstrate that the game is ignoring the specified parameters like you said. Good luck.

 

dude. @Dunto. you're a forum moderator, right? not one of the developers?

lexyeevee is a professional programmer (= a company pays her real human money to write code) with a fair bit of clout and I think it's safe to say she knows what she's talking about. maybe don't talk down to her like she doesn't know what a list is?

at the very least maybe ask one of the actual devs what they think.

 

I'm sure lexyeevee knows what they're talking about. I'm merely trying to point out that just because someone does not like the structure does not automatically make it invalid code. I won't be bothering the devs about it, however.
@lexyeevee If you were offended, then I apologize, it was not my intention to patronize you. I appreciate the time you took to report this, I just wanted you to understand that if it doesn't break anything nor is likely to break anything down the road, the chances of it being changed in the immediate future is quite slim. That's all. Demonstrating why you think it's an issue may help with prioritizing a fix.

(@eris.control While I usually give the benefit of the doubt, on the internet anyone can claim anything. I'm glad people have real-world clout, but that means little over the internet. )

 

I don't have time to squabble with a gatekeeper who can't tell the difference between personal preference and an obvious schema violation. The issue is trivial and the patch is available, should the developers want it.

 

I linked the dev team to this post-- Eevee was exactly right. Thanks for the report. However...

This sort of attitude is seriously unacceptable here. Dunto wasn't trying to "squabble" with you, and when it became clear you took offense he was genuinely apologetic. I can understand getting a bit annoyed, but I think your response was over the top.

 

@Dunto hehhh can't really argue with 5k twitter followers, or the fact that an article she wrote back in 2012 (with 2380 comments) ranks #1 on google and #4 on duck duck go if you search "php criticism" and happens to be one of the most frequently cited articles on the subject. I mean maybe that's not enough to warrant a doctorate but it would seem to imply that folks value what she has to say.

@mollygos "sorry if you were offended" isn't an apology imo.

Anyways, the important thing here is that the issue got fixed.