Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

line-of-sight observation format is inconsistent #355

Closed
TGlas opened this issue Sep 16, 2016 · 3 comments · Fixed by #357
Closed

line-of-sight observation format is inconsistent #355

TGlas opened this issue Sep 16, 2016 · 3 comments · Fixed by #357
Assignees
Milestone

Comments

@TGlas
Copy link

TGlas commented Sep 16, 2016

The format of the LineOfSight (ray) observations is inconsistent, mixing false (proper json boolean) and "false" (json string) for booleans. Example:

 "LineOfSight":{
  "check_decay":"false",   // string
  "decayable":"true",      // string
  "hitType":"block",
  "inRange":false,         // boolean
  "type":"leaves",
  "variant":"oak",
  "x":240,
  "y":67.6200000047684,
  "z":143.706234534943
 },

I'd prefer proper json booleans everywhere. Please fix this before too many people start to rely on the strings. Also, some boolean properties seem to appear only if they are true, i.e., "snowy" exists but is missing from the above example.

@TGlas
Copy link
Author

TGlas commented Sep 16, 2016

There is an even more serious issue with these observations. I just stumbled across the following one:

 "LineOfSight":{
  "hitType":"block",
  "inRange":true,
  "type":"tall_grass",            // inconsistent name!
  "x":244.511514540456,
  "y":67.7240803828614,
  "z":143.899999976158
 },

Note the "tall_grass" value. In my Types.xsd file the name of the block type is tallgrass, i.e., without underscore. This makes a lookup in my code fail. Please keep the block names consistent, otherwise they are rather useless.

@DaveyBiggers DaveyBiggers added this to the Fallow Deer milestone Sep 16, 2016
@DaveyBiggers
Copy link
Member

Good spot! The code that builds the json adds a property called "type" with the blocktype, and the "inRange" property, etc.etc. It then adds the Minecraft block properties, as they are named in Minecraft - "decayable" and "check_decay" are examples of these - and it adds their values as strings. This is why "inRange" is a proper bool, and "decayable" isn't. This is also why some properties don't always appear. "snowy" does still appear when it's false, but only if you are looking at a block which has the snowy property - eg grass blocks have the snowy property, but leaves don't.

The boolean problem can, should, and will be fixed - thank you for pointing it out. The appearing/disappearing properties isn't a bug, and won't be fixed.

More seriously, it turns out that some blocks have sub-types, and they store this sub-type in a property called... "type" - which then overwrites the type already added. This is what is happening in the tallgrass/tall_grass case. The type of the block is correctly set to "tallgrass", but then overwritten by the block's own "type" property - which, in this case, will be one of "dead_bush", "tall_grass" or "fern". (I suppose the Minecraft folks just decided that all block sub-types would use this_naming_convention, whereas block types would usethisnamingconvention.)

I shall fix this by prefixing the word "prop_" to each of these block properties. Eg your second example will become:

LineOfSight":{
  "hitType":"block",
  "inRange":true,
  "type":"tallgrass",
  "prop_type":"tall_grass", // (or "dead_bush" or "fern")
  "x":244.511514540456,
  "y":67.7240803828614,
  "z":143.899999976158
 },

and the first example will become:

"LineOfSight":{
  "prop_check_decay":false,   // boolean
  "prop_decayable":true,      // boolean
  "hitType":"block",
  "inRange":false,            // boolean
  "type":"leaves",
  "variant":"oak",
  "x":240,
  "y":67.6200000047684,
  "z":143.706234534943
 }

@TGlas
Copy link
Author

TGlas commented Sep 21, 2016

Sorry, it took me a while to get back to this. Thanks for the fix, now it works perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants