ClassiCube icon indicating copy to clipboard operation
ClassiCube copied to clipboard

CC is too picky about integer variants in CW file.

Open rdebath opened this issue 2 years ago • 2 comments

If it "expects" a 16 bit integer it throws it's toys away if it gets an eight bit integer. While a too big integer value is a reasonable reason to get testy, this seems far too picky.

(NB: MCGalaxy too)

Crash time: 06/09/2022 21:04:34
ClassiCube crashed.
Reason: Expected I16 NBT tag
-- backtrace --
00005621EB94C7E7 - ClassiCube
00005621EB96F871 - ClassiCube
00005621EB9703E5 - ClassiCube
00005621EB970608 - ClassiCube
00005621EB970608 - ClassiCube
00005621EB970608 - ClassiCube
00005621EB970608 - ClassiCube
00005621EB97074C - ClassiCube(Cw_Load+124)
00005621EB970831 - ClassiCube(Map_LoadFrom+65)

rdebath avatar Sep 06 '22 20:09 rdebath

Making NBT tag parsing more resilient is something I've thought about a while

But my concern with relaxing requirements is that other software might generate .CW files that although would now be accepted, would also hard crash older stricter client versions

UnknownShadow200 avatar Sep 06 '22 22:09 UnknownShadow200

Other software breaking something because they write what you read rather than what you write really doesn't seem like your problem ...

In fact I suspect this should be an application for "Postal's law", specifically it's something like: "be conservative in what you send, be liberal in what you accept"

That way if someone is using you as a pattern they should check what you write to see what they should write not what you will read. (If you are looking at the web browsers, IMO they should be throwing out hundreds of warnings for most websites -- to the dev console of course; nobody else would be interested)

rdebath avatar Sep 07 '22 08:09 rdebath

I16/U16/I32 tag retrieval functions will now also support 8 and 16 bit integer tags

(NB: MCGalaxy too)

What issue is there with MCGalaxy's importer? It should already be more lenient compared to the client's

UnknownShadow200 avatar Oct 13 '22 07:10 UnknownShadow200

MCG is more lenient, but still has occasional issues.

  • Unused fields are not completely ignored; if one is duplicated it will cause the importer to fail.
  • If a used field has an integer type that is too large an error is thrown.
  • Previous happens even if the field contains a value in range
  • Previous happens even if the field is optional.
  • If a used, optional, field is of the wrong basic type (float vs integer vs string) it still throws an error.
  • Optional fields are usually considered mandatory when their container exists so Spawn must have all five values even though H and P would reasonably default to zero. (Also many fields in the Blockdefs)

For me, only the first one has tripped me for something that was not an actual error.

The other issues basically come from me using XML/CSV/YAML all the time where the "type" of a field is purely defined by it's value.

rdebath avatar Oct 28 '22 08:10 rdebath