[Spec] Additions, Removals, and Deprecations #76

hexabits · 2018-06-04T15:10:46Z

This is a catch-all for attribute changes and their discussion that are not covered by other tickets.

Replace/Remove/Rename

General

For any attributes intended as boolean, true/false should be used over 1/0. They are valid in XML as well, and more clear as to their intent for vague attribute names.
Replace TEMPLATE and ARG with #T# and #ARG#, to match with [Spec] Token definitions #70

'Flags' type removal

As discussed in #3, a replacement for Flags is desperately needed. Instead of rewriting the issue of that ticket I will put everything here with the rest of the removals and changes.

While the comments of issue #3 were very informative for the concept and possible solutions, we were all trying to change <bitflags> to suit our needs. A bitflag is an enumeration, and in languages like C++ is able to be represented with an enum, except instead of 0,1,2,3, the values go 0x1,0x2,0x4,0x8, just like we are doing with our enum and bitflags.

What we were really discussing was bitfields, which are separate from enumerations. They are simply a struct, but with a bit spanning syntax. In my 010 editor templates (C-like), a bitfield looks like this:

typedef union {
    ushort value;
    struct {
        ushort doBlending      : 1;
        BlendFunction srcBlend : 4;
        BlendFunction dstBlend : 4;
        ushort doTesting       : 1;
        TestFunction testFunc  : 3;
        ushort noSorter        : 1;
    } bits;
} AlphaFlags;

A bitflag can also be represented in this way, but only with : 1 at the end of each statement. (I will note that in 010 editor I also used bitfields for bitflags, but simply due to the way the UI treats enums.)

The current proposed syntax is:

<bitfield name="TimeControllerFlags" storage="ushort">
    Flags for NiTimeController
    <member width="1" pos="0" mask="0x0001" name="Anim Type" type="AnimType" />
    <member width="2" pos="1" mask="0x0006" name="Cycle Type" type="CycleType" default="CYCLE_CLAMP" />
    <member width="1" pos="3" mask="0x0008" name="Active" type="bool" default="1" />
    <member width="1" pos="4" mask="0x0010" name="Play Backwards" type="bool" />
    <member width="1" pos="5" mask="0x0020" name="Manager Controlled" type="bool" />
    <member width="1" pos="6" mask="0x0040" name="Compute Scaled Time" type="bool" default="1" />
    <member width="1" pos="7" mask="0x0080" name="Forced Update" type="bool" />
</bitfield>

For C/C++ and other languages with actual bitfields, only the name, storage="ushort", and width are needed. The type shown here for each member is NOT used for (de)serialization, it is more or less the return type for getters and the input type for setters for methods to set the members.

For anything else that cannot use just the width for each member, the position and mask have been included. These allow you to retrieve the value for the member from the ushort directly.

Getter

(flags & MASK) >> POS

Setter

flags = (flags & ~MASK) | (value << POS)

Where flags is the entire ushort, and value is the value of the particular field.

<add>

Remove: `userver` and `userver2`

These two attributes had minimal remaining use. I've already switched their use to the format vercond="User Version == ...". Also, userver2 was never technically formalized afaict. It was not supported in every project such as niflib, where nifxml.py/gen_niflib.py seem to have no support for it.

Remove: `arr3`

There is a single legacy undecoded object using arr3 for some reason, and it is otherwise undocumented and unsupported by all programs. It only seems to exist due to lack of arithmetic in the arr expr, which exists now. So it is being removed in lieu of that.

Rename: `ver1` and `ver2`

For block versioning, I have adopted since and until and this is succinct and human readable. The nice would be the same for <add>. However there is also alternatives like minver and maxver. But ver "1" and ver "2" inherently mean nothing.

Replace/Remove: `calculated`

I've talked about this attribute a lot and still am completely in the dark about its intent. If it does what I think it's intended for, it should be on way more fields than the 3 that it is present on.

For most things, there is a simple two-way boolean relationship for example:

<add name="Has Triangles" type="bool" calculated="1" />
<add name="Triangles" type="Triangle" arr1="Num Triangles" cond="Has Triangles" />

I don't really understand the need for calculated here. At write time, if the Triangles array has a length, you know that you must set Has Triangles to true. If the Triangles array has no length, it doesn't matter whether or not Has Triangles is set to true, unless for some reason the program has erroneously set a Num Triangles but doesn't have any Triangles in the array.

Where it's actually needed is in examples like this:

<add name="Vertex Desc" type="BSVertexDesc" />
<add name="Num Triangles" type="ushort" />
<add name="Num Vertices" type="ushort" />
<add name="Data Size" type="uint" calculate="(Num Vertices * (Vertex Desc &amp; 0xF) * 4) + (Num Triangles * 6)" />
<!-- -->
<add name="Particle Data Size" type="uint" calculate="(Num Vertices * 6) + (Num Triangles * 3)" />

You can calculate the Data Size values from the content after them, but in this case, the data before Data Size has to be in agreement as well.

Except... either size can also be 0 even if Vertex Desc, Num Triangles, and Num Vertices are all non-zero. So, really the calculate field would only be a guideline on how to get the correct value, not a rule that is always followed.

Replace `abstract`

NiDataStream has two members which do not contribute to serialization. This is done because NiDataStream RTTI string has two arguments packed at the end so they are effectively the first two members of the NiObject, just packed with the name instead of the block.

The name abstract is confusing for many reasons, but it's simply factually incorrect in OOP, which the XML is emulating.

As for alternatives, hkxcmd uses flags='SERIALIZE_IGNORED' for example, but it also has a lot of flags for its class members. I also think using an attribute would be kind of hard to see and a separate tag name would also enforce treating them differently so that there is no mistaking that the data need not be serialized.

So, the obvious choices would be to have tags named <property>/<prop> or <metadata>/<meta>.

For existing parsers or codegen, this has the benefit of not confounding members that serialize vs members that don't, as they simply will not see the new tag. Right now, anything that doesn't pay attention to abstract on members will erroneously try to read/write the values from disk.

Replace `cond="T"` and `cond="!T"`

Several instances of cond involve checking against the type of NiObject the member is being included in such as

<niobject name="NiExtraData" inherit="NiObject">
    <add name="Name" type="string" cond="!BSExtraData">
    <!-- -->
</niobject>

or for NiGeometry

<add name="Bound" type="NiBound" vercond="(User Version 2 &gt;= 100)" cond="NiParticleSystem" />
<add name="Skin" type="Ref" template="NiObject" vercond="(User Version 2 &gt;= 100)" cond="NiParticleSystem" />
<!-- -->
<add name="Material Data" type="MaterialData" vercond="(User Version 2 &lt; 100)" />
<add name="Material Data" type="MaterialData" vercond="(User Version 2 &gt;= 100)" cond="!NiParticleSystem" />

Mostly in order to fix inheritance issues caused by historical changes to class hierarchies, or in Bethesda's case, entire class replacement (NiGeometry->BSGeometry). Since we have no mechanism for multiple niobject definition with inheritance changes, we settle for customizing the content of the parent niobject based on the child type.

This mechanism is not an expression though, and as such does not belong inside of cond. The names of NiObject types should not appear in cond as they are not local information.

The replacement would be:

<niobject name="NiExtraData" inherit="NiObject">
    <add name="Name" type="string" excludeT="BSExtraData">
    <!-- -->
</niobject>

or for NiGeometry

<!-- BSGeometry: Used by Bethesda Stream 100+ NiParticleSystem -->
<add name="Bound" type="NiBound" vercond="(User Version 2 &gt;= 100)" onlyT="NiParticleSystem" />
<add name="Skin" type="Ref" template="NiObject" vercond="(User Version 2 &gt;= 100)" onlyT="NiParticleSystem" />
<!-- -->
<add name="Material Data" type="MaterialData" vercond="(User Version 2 &lt; 100)" />
<add name="Material Data" type="MaterialData" vercond="(User Version 2 &gt;= 100)" excludeT="NiParticleSystem" />

Two separate attributes will set it apart from normal cond, which will no longer have to be parsed to see whether or not any part of the expression is an NiObject and whether there is a unary not (!).

For existing applications, if rewriting logic is undesirable, excludeT and onlyT can simply be aliases for cond="!T" and cond="T" respectively.

<basic>

Rename `count`

The function of this initially eluded us until I saw the connection between the types that have it, and then eventually noticed that the language in the docs generator supported this. Basically, it means "integral", but should mean "countable". It doesn't technically mean countable because the int basic type is set as countable as well, even though signed ints should not be used as counts.

The phrase "count" is also too close in meaning to "size" or "length", which may also be introduced as an attribute for basic/compound types, and the use of 0/` instead of false/true also further confuses the intent.

Additions

<basic> and <compound>

`int64` and `uint64` types

A few types in the XML are really bitfields with a 64-bit storage type, and at least one field is potentially a 64-bit hash. So the addition of 64-bit types is necessary.

`size`

The basic and basic-ish compound types have no information on their size for read/write. Objects that are always a fixed size should declare their size in the XML.

`casts_to` / `convertible`

The other types that it is acceptable to be cast to (where information is not lost), e.g. short->int or ushort->uint. This is necessary for deducing if version-exclusive duplicate names of differing types are true duplicates or not. Or if the members are in an incorrect order, e.g. for ushort->uint, the uint has to be listed first.

It also is useful for defining what compound types must have a conversion function available, such as HalfVector3 -> Vector3.

For example:

<add name="Num Triangles" type="uint" vercond="User Version 2 == 130" />
<add name="Num Triangles" type="ushort" vercond="User Version 2 &lt; 130" />

These do not need to be treated as different values for codegen or anything else because a ushort can be held inside of a uint. However if the members were in reverse order, the uint could not be fit into the ushort without potential information loss.

For Python, this doesn't really matter much, as there is no real size limit to integers, but it does affect the logic during (de)serialize. It will also aid in linting.

Example of usage:

<basic name="byte" countable="true" casts_to="ushort uint">
<basic name="char" countable="false" casts_to="short int">

The opposite direction of this could also be used instead

<basic name="uint" countable="true" casts_from="ushort byte">
<basic name="int" countable="false" casts_from="short char">

This would basically be a list of all the types that can fit inside it.

If the basic types also had their size attribute, this might not even be necessary.

Split `countable` into `integral`, `boolean`, and `countable`

Currently the signed integer values are marked as countable, but really shouldn't be. You can't have a negative value for an array length. The type is still integral. Furthermore, it's not boolean. The only types that should be considered boolean are byte and bool. Boolean types should be the only types that can toggle the presence of a sibling member. Adding integral and boolean will help sort out typing issues during linting for array and cond references.

In Summary

Integral is not necessary countable
Signed integers should not be countable (used for array sizes)
Integral == False implies countable and boolean are false as well.
Boolean == True should be the only way a member can toggle the presence of another member.

<niobject>

Defaults of inherited values

For each niobject, there can be only one default for a member, in the niobject that it is declared on. However, several Gamebryo classes modify inherited defaults for each class type. Some examples:

NiPSysModifier\Order: Specific subclasses of NiPSysModifier always have the same Order
NiAVObject\Flags: Subclasses of NiAVObject tend to have different Flags defaults, such as NiNode vs BSFadeNode vs BSBlastNode/BSDamageStage.

So to provide proper defaults for subclasses, a syntax is necessary for overriding default values of inherited members.

Using a Pythonic dictionary syntax:

<niobject name="NiPSysModifier" abstract="true" inherit="NiObject">
    Abstract base class for all particle system modifiers.
    <!-- -->
    <add name="Order" type="uint">Modifier ID in the particle modifier chain</add>
    <!-- -->
</niobject>

<niobject name="NiPSysEmitter" abstract="true" inherit="NiPSysModifier" defaults="{'Order': 1000}">
    <!-- -->
</niobject>

<niobject name="NiPSysDragModifier" inherit="NiPSysModifier" defaults="{'Order': 4000}">
    <!-- -->
</niobject>

Downside: Other than Python using literal eval, this syntax would have to be specially parsed. A native XML syntax could be used, but it would be more verbose and introduce more tag names.

Example:

<niobject name="NiPSysModifier" abstract="true" inherit="NiObject">
    Abstract base class for all particle system modifiers.
    <!-- -->
    <add name="Order" type="uint">Modifier ID in the particle modifier chain</add>
    <!-- -->
</niobject>

<niobject name="NiPSysEmitter" abstract="true" inherit="NiPSysModifier">
    <default name="Order" value="1000" />
    <!-- -->
</niobject>

<niobject name="NiPSysDragModifier" inherit="NiPSysModifier">
    <default name="Order" value="4000" />
    <!-- -->
</niobject>

Downside: NifSkope currently actually errors if anything other than add exists inside of niobject etc. so simply adding a new child tag would not be ignored. This is a minor downside, and can quickly be fixed, but just pointing out that new tags are not necessarily ignored by default.

Organizationally, a heterogeneous mix of child tags is a bit odd for nif.xml but not XML in general. The default metadata could also be supplied alongside each subclass, or in the parent itself.

In the parent:

<niobject name="NiPSysModifier" abstract="true" inherit="NiObject">
    Abstract base class for all particle system modifiers.
    <!-- -->
    <add name="Order" type="uint">
        Modifier ID in the particle modifier chain
        <default type="NiPSysEmitter" value="1000" />
        <default type="NiPSysDragModifier" value="4000" />
        <!-- -->
    </add>
    <!-- -->
</niobject>

Upside: This centralizes the info and it's not strewn across the subclasses.
Downside: This info is not apparent when looking at a subclass. 🤕

Note: For this particular member, there may be more than a dozen actual defaults listed.. not just the two shown in the examples.

Lastly, I mentioned that it's technically metadata i.e. not directly vital to (de)serialization which is true, but it is still vital to data correctness. That is why the last option of putting the info alongside the niobjects instead of inside is the least appealing to me. It would essentially be a type-name-value dictionary in XML so I won't bother with an example.

<add>

`range` (numeric limits)

As of writing this, I use calc in a few locations to limit the final size of a Num value. This is fine, but in the general case, some values need to be enforced to a very limited range of their full storage size. In some cases (such as Num values), the only limitation is that the value must be at least 1. Sometimes it is an upper limit. I feel that having an attribute for both the lower and upper limit is excessive and instead the Python range syntax can be borrowed.

Right now, I've begun marking some of these fields as default="1" because there should always be at least one, but this isn't actually an enforcement.

<!-- Min+Max -->
<add name="Num A" type="uint" range="1:8" default="1" />
<!-- No Max -->
<add name="Num B" type="uint" range="1:" default="1" />
<!-- No Min -->
<add name="Num C" type="uint" range=":255" />

For the first two, default="1" is technically redundant. Since the default for a uint is 0, then any min of non-zero automatically becomes the default. However, existing systems already rely on the default attribute for member initialization, and the number of fields that will have default="1" range="1:" should not be too high. This opinion may change though.

To not write both, any non-zero min of range="X:Y" would have to imply default="X" and any parser would have to derive default="X" on its own.

`const` (immutable values)

Some class members should be set at creation time and be immutable, i.e. not changeable by a user or any other means. For example type info that is tightly coupled with class type.

A default value is obviously required when const="true".

<enum> and <bitflags>

Bit Patterns in bitflags

Several bitflags have default attributes set with plain integral values, and this doesn't allow anyone to easily see which bitflags options these are. For example:

<add name="Shader Flags 1" ... default="2185233153" />
<add name="Shader Flags 2" ... default="32801" />
<add name="Shader Flags 1" ... default="2151678465" />
<add name="Shader Flags 2" ... default="1" />

Default fields don't support expressions, and so a token with the various options BITORed together would not help. It could instead be a feature of the option:

<option value="0|8|9|22|25|31" name="SLSF1_DEFAULT" />

<add name="Shader Flags 1" ... default="SLSF1_DEFAULT" />

Or so. Each value would get split on | and then shifted and BITORed. This way, using actual enumeration values as defaults could be enforced for bitflags as well.

Clarifications

Some things remain unclear about nif.xml and need to be stated explicitly

<add>

`default` usage for arrays

Nowhere is default used alongside arr1. However there are some arrays where a default should be provided. The default would apply to any members added to the array of that type.

Example:

<add name="Vertex Colors" type="Color4" default="#VEC4_ONE#" arr1="Num Vertices" />

The default color for the Vertex Colors array should be 1.0, 1.0, 1.0, 1.0.

For Discussion

Like other tickets, this isn't completely finished and I may be adding more as time goes on. I will be making commits for this ticket fairly immediately however.

Checklist

The text was updated successfully, but these errors were encountered:

The `ver` attribute was never actually formalized and just meant as a shorcut for using both ver1/ver2 on the same version. `userver` was almost never used and `userver2` was becoming increasingly uncommon as most comparisons were switched to inequalities. To make things easier, these should just be part of the expression.

…ols#76 The `cond` attribute was being overloaded with multiple functions. Conditions should be actual expressions, using local or inherited data. Type checking inside of `cond` would also mean that you could do both in one expression, such as `cond="NiParticleSystem && Num Vertices > 0"` which doesn't make any sense. So, `onlyT` and `excludeT` were made to serve that function. Two attributes also means not having to use the unary not (!) in order negate the meaning. Existing logic can simply alias the attributes to cond if need be.

For niftools#76 `bool` type is something that has to be dealt with as it changes size. Also it's marked as countable which should probably be analyzed... It shouldn't be used in array counts.

For niftools#76

Was completely unsupported and undocumented and unused by any software. Replaced with arithmetic even though the decoding is likely wrong anyway. For niftools#76

For niftools#70 and niftools#76 Adopting the token syntax of the other tokens, TEMPLATE and ARG can now be treated the same as other tokens and differentiated from regular identifiers in a generic manner.

For niftools#76 Also reorganized the basic types to be in a logical order.

For niftools#76 The signed integeral types were marked as countable even though they should not be used for array sizes. A negative value cannot be passed into an array size. They are still integral though, so countable was split into `countable` and `integral`. In addition, added `boolean` to designate types that can toggle the presence of another member. Also reordered items for `convertible` so that the referenced types come before the type referencing it, i.e. larger types first.

For niftools#74 Implemented thanks to changes done in niftools#76 for differentiating integral and "countable" i.e. can be used for sizes. Marking types as boolean was also in niftools#76, yet there were no errors for this lint; all the `cond` references that were not complex expressions were already boolean types.

For niftools#3, niftools#76 Replaces all Flags types with real enums (where there were only two options) or a new bitfield type. Bitfield mimics the bitfield syntax of C structs (Compounds), but also acts like Enum/Bitflags with the `storage` attribute in that the entire structure can simply be aliased to a basic type like `ushort`. The `type` on each Bitfield member does **NOT denote size**. Each member only takes up the bits that are defined by `width` and `pos`. The `type` information is for casting for getters and setters. That means for C structs, code generation would not use the type on each member, but the storage type instead.

hexabits · 2018-06-14T19:36:44Z

I won't repeat the entire thing here, but I detailed the usage of bitfield in the original ticket: #3 (comment)

For niftools#76 For reduced ambiguity. Adapting for this in Python is as easy as ```py # XML Booleans XML_TRUE = ["true", "1"] XML_FALSE = ["false", "0"] # Example self.is_abstract = get('abstract') in XML_TRUE ``` Note "True" and "False" are not valid booleans in XML. Only lowercase.

For niftools#76. `calc` does not need to be supported and is only used for pre-serialization preparation of the data. This can also be done by hand. Some revisions also need to be made for niftools#70 and niftools#73 to account for another attribute with its own expression grammar and tokens. However, the tokens introduced for `calc` will not be added to the tokens in the XML and will need to be explicitly supported if supporting `calc`. They are: 1D array size (uses regex): `#LEN[(.*?)]#` 2D array size (uses regex): `#LEN2[(.*?)]#` Ternary `?`: `#THEN#` Ternary `:`: `#ELSE#` Also removes any unnecessary `calculated` without replacement. Additionally, `calc` is used to limit array size for the time being, though it's possible this should be done with its own attribute, such as `maxlen`.

hexabits · 2018-07-20T18:16:34Z

I added a section titled Defaults of inherited values for niobject.

My thoughts:

The attribute method is easier to implement and ignore, but harder to actually support. The element method is strictly XML with no special parsing required and as child elements of <add> anything existing should ignore it. The element method is a lot more verbose for members with overrides in many subclasses, but doesn't require a dictionary syntax parser.

I'm guessing I will implement it as the element method. As for the defaults being centralized instead of associated with the subclass, I can always document the defaults in the docstring.

hexabits · 2018-07-20T19:13:07Z

Also one problem with the centralized declaration I just thought of is that for

    <add name="Order" type="uint">
        Modifier ID in the particle modifier chain
        <default type="NiPSysEmitter" value="1000" />
        <default type="NiPSysDragModifier" value="4000" />
        <!-- -->
    </add>

The type are not declared until later, so this can become a forward declaration problem depending on language/implementation. Forward declarations are not something that nif.xml has done yet, which is in particular a problem with recursion and declaration order in the XML.

However in this case, the data here is metadata. It is not actually required for the class it is declared in, but stored and used later on for the declaration of its subclasses.

Also not handled in the issue text is the declaration of the base default. I suppose either of these works, and maybe both should be valid:

    <add name="Order" type="uint"

hexabits added a commit to hexabits/nifxml that referenced this issue Jun 5, 2018

Add size to compounds

04fe24d

For niftools#76

hexabits added a commit to hexabits/nifxml that referenced this issue Jun 6, 2018

Remove arr3 from legacy object

29d0245

Was completely unsupported and undocumented and unused by any software. Replaced with arithmetic even though the decoding is likely wrong anyway. For niftools#76

hexabits mentioned this issue Jun 7, 2018

[Spec] nifxml linter #74

Open

hexabits added a commit to hexabits/nifxml that referenced this issue Jun 7, 2018

Add int64 and uint64

f64419a

For niftools#76 Also reorganized the basic types to be in a logical order.

hexabits mentioned this issue Jun 14, 2018

Investigate depreciating <basic name ="Flags"> #3

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Spec] Additions, Removals, and Deprecations #76

[Spec] Additions, Removals, and Deprecations #76

hexabits commented Jun 4, 2018 •

edited by Candoran2

Loading

hexabits commented Jun 14, 2018

hexabits commented Jul 20, 2018

hexabits commented Jul 20, 2018

[Spec] Additions, Removals, and Deprecations #76

[Spec] Additions, Removals, and Deprecations #76

Comments

hexabits commented Jun 4, 2018 • edited by Candoran2 Loading

Replace/Remove/Rename

General

'Flags' type removal

<add>

Remove: userver and userver2

Remove: arr3

Rename: ver1 and ver2

Replace/Remove: calculated

Replace abstract

Replace cond="T" and cond="!T"

<basic>

Rename count

Additions

<basic> and <compound>

int64 and uint64 types

size

casts_to / convertible

Split countable into integral, boolean, and countable

In Summary

<niobject>

Defaults of inherited values

<add>

range (numeric limits)

const (immutable values)

<enum> and <bitflags>

Bit Patterns in bitflags

Clarifications

<add>

default usage for arrays

For Discussion

Checklist

hexabits commented Jun 14, 2018

hexabits commented Jul 20, 2018

hexabits commented Jul 20, 2018

hexabits commented Jun 4, 2018 •

edited by Candoran2

Loading

Remove: `userver` and `userver2`

Remove: `arr3`

Rename: `ver1` and `ver2`

Replace/Remove: `calculated`

Replace `abstract`

Replace `cond="T"` and `cond="!T"`

Rename `count`

`int64` and `uint64` types

`size`

`casts_to` / `convertible`

Split `countable` into `integral`, `boolean`, and `countable`

`range` (numeric limits)

`const` (immutable values)

`default` usage for arrays