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 #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
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 & 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 >= 100)" cond="NiParticleSystem" />
<add name="Skin" type="Ref" template="NiObject" vercond="(User Version 2 >= 100)" cond="NiParticleSystem" />
<!-- -->
<add name="Material Data" type="MaterialData" vercond="(User Version 2 < 100)" />
<add name="Material Data" type="MaterialData" vercond="(User Version 2 >= 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 >= 100)" onlyT="NiParticleSystem" />
<add name="Skin" type="Ref" template="NiObject" vercond="(User Version 2 >= 100)" onlyT="NiParticleSystem" />
<!-- -->
<add name="Material Data" type="MaterialData" vercond="(User Version 2 < 100)" />
<add name="Material Data" type="MaterialData" vercond="(User Version 2 >= 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 < 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