Investigate depreciating <basic name ="Flags">

The basic type Flags just a wraps a number value but offers no value. The number in most cases is still presented to the user as is, and the options, usually documented in the nif.xml is unlikely to read by a user .

There are two structures which we currently use which provide what the "Flags" real functionality should.

- where only one "flag" value is selectable, eg different shader types.
a combination of "flags" value

<add name="Flags" type="Flags">
   Controller flags (usually 0x000C). Probably controls loops.
   Bit 0 : Anim type, 0=APP_TIME 1=APP_INIT
   Bit 1-2 : Cycle type  00=Loop 01=Reverse 10=Loop
   Bit 3 : Active
   Bit 4 : Play backwards
</add>

This could be reworked as

<bitflag name="AnimationFlags">
Controller flags (usually 0x000C). Probably controls loops.
    <option value="0" name="Anim type">APP_TIME</option>
    <option value="1" name="Anim type">APP_INIT</option>
    <option value="3" name="Cycle Type Loop">Loop</option>
    <option value="4" name="Cycle Type Reverse">Reverse</option>
    <option value="5" name="Cycle Type">Loop</option>
    <option value="7" name="Active">Active</option>
    <option value="8" name="Play backwards">Play backwards</option>
</bitflag>

Then the above would be simplified to this
<add name="Flags" type="AnimationFlags"></add>

This example does highlight a potential redesign in implementation. We do not have any way to to allow certain flags to be mutually exclusive, only be able to select subset of flags, ie only one Cycle Type selectable.

Additionally most tools have to add additional logic so that the value is presented in a meaningful manner to the user. eg. NifSkope know to break Col Filter into 3 options, LINK property, Collision, *Scaled

5 bits to represent the remaining options.

A partial solution, but using bitflags a generic implementation would just pull all the information directly. Less code to maintain if its generic.

The only downside is that it more bitflag types will need to be created and depending on the number of available options this will increase the size of the nif.xml. If the options can be conditional then some could be reused, like Col Filter & Skyrim Col Filter, but less readable.

@neomonkeus @ttl269 I think I've arrived at this conclusion as well. The values for the Flags end up being hardcoded in NifSkope and then do not get updated when nif.xml is updated.

However I think the proposed formatting isn't sufficient. A more complicated example is:

Bit 0 : alpha blending enable
Bits 1-4 : source blend mode
Bits 5-8 : destination blend mode
Bit 9 : alpha test enable
Bit 10-12 : alpha test mode
Bit 13 : no sorter flag ( disables triangle sorting )

blend modes (glBlendFunc):
0000 GL_ONE
0001 GL_ZERO
0010 GL_SRC_COLOR
0011 GL_ONE_MINUS_SRC_COLOR
0100 GL_DST_COLOR
0101 GL_ONE_MINUS_DST_COLOR
0110 GL_SRC_ALPHA
0111 GL_ONE_MINUS_SRC_ALPHA
1000 GL_DST_ALPHA
1001 GL_ONE_MINUS_DST_ALPHA
1010 GL_SRC_ALPHA_SATURATE

test modes (glAlphaFunc):
000 GL_ALWAYS
001 GL_LESS
010 GL_EQUAL
011 GL_LEQUAL
100 GL_GREATER
101 GL_NOTEQUAL
110 GL_GEQUAL
111 GL_NEVER

It would be nice if the format would be able to reflect that certain bits are grouped together and are exclusive options within that bit range.

If this can't be done strictly in XML for whatever reason, the text inside the current tags could at least be in a more easily parsable format.

@jonwd7 @neomonkeus Proposal of bitflags enhancement to be universal for any flags:

<bitflags name="AlphaPropertyFlags" storage="ushort">
    Transparency flags.
    <option value="0" name="Enable Blending">alpha blending enable</option>
    <bitgroup value="1 4" name="Source Blend Mode">source blend mode
        <option value="0000" name="One">GL_ONE</option>
        <option value="0001" name="Zero">GL_ZERO</option>
        <option value="0010" name="Src Color">GL_SRC_COLOR</option>
        <option value="0011" name="Inv Src Color">GL_ONE_MINUS_SRC_COLOR</option>
        <option value="0100" name="Dst Color">GL_DST_COLOR</option>
        <option value="0101" name="Inv Dst Color">GL_ONE_MINUS_DST_COLOR</option>
        <option value="0110" name="Src Alpha">GL_SRC_ALPHA</option>
        <option value="0111" name="Inv Src Alpha">GL_ONE_MINUS_SRC_ALPHA</option>
        <option value="1000" name="Dst Alpha">GL_DST_ALPHA</option>
        <option value="1001" name="Inv Dst Alpha">GL_ONE_MINUS_DST_ALPHA</option>
        <option value="1010" name="Src Alpha Saturate">GL_SRC_ALPHA_SATURATE</option>
    </bitgroup>
    <bitgroup value="5 8" name="Destination Blend Mode">destination blend mode
        <option value="0000" name="One">GL_ONE</option>
        <option value="0001" name="Zero">GL_ZERO</option>
        <option value="0010" name="Src Color">GL_SRC_COLOR</option>
        <option value="0011" name="Inv Src Color">GL_ONE_MINUS_SRC_COLOR</option>
        <option value="0100" name="Dst Color">GL_DST_COLOR</option>
        <option value="0101" name="Inv Dst Color">GL_ONE_MINUS_DST_COLOR</option>
        <option value="0110" name="Src Alpha">GL_SRC_ALPHA</option>
        <option value="0111" name="Inv Src Alpha">GL_ONE_MINUS_SRC_ALPHA</option>
        <option value="1000" name="Dst Alpha">GL_DST_ALPHA</option>
        <option value="1001" name="Inv Dst Alpha">GL_ONE_MINUS_DST_ALPHA</option>
        <option value="1010" name="Src Alpha Saturate">GL_SRC_ALPHA_SATURATE</option>
    </bitgroup>
    <option value="9" name="Enable Testing">alpha test enable</option>
    <bitgroup value="10 12" name="Destination Blend Mode">destination blend mode
        <option value="000" name="Always">GL_ALWAYS</option>
        <option value="001" name="Less">GL_LESS</option>
        <option value="010" name="Equal">GL_EQUAL</option>
        <option value="011" name="Less or Equal">GL_LEQUAL</option>
        <option value="100" name="Greater">GL_GREATER</option>
        <option value="101" name="Not Equal">GL_NOTEQUAL</option>
        <option value="110" name="Greater or Equal">GL_GEQUAL</option>
        <option value="111" name="Never">GL_NEVER</option>
    </bitgroup>
    <option value="13" name="No Sorter">no sorter</option>
    <option value="15" name="Editor Alpha Ref Test">editor alpha ref test - function unknown</option>
</bitflags>

Introduced new xml element bitgroup for defining of group of bits inside bitflags element. Its parameter value would define range of bits. In my proposal the starting and ending bits are separated by space (is it suitable for parsing?). Then any element option used inside bitgroup would define desired bit combinations using characters 0 and 1 in value parameter. Other possible way to define a range of bits for bitgroup element could be:

<bitgroup bitstart="1" bitend="4" name="XXX">

Which maybe better for parsing the values defining the range of bits.

It would be great if Nifskope could parse this enhanced bitflags and mainly visually represents them. For example this use of bitflags:

<bitflags name="SomeFlags" storage="ushort">
    Flags flags flags.
    <option value="0" name="Enable XXX">decription of XXX enabled</option>
    <option value="2" name="Enable YYY">decription of YYY enabled</option>
    <bitgroup value="3 6" name="ZZZ Mode">description of ZZZ mode
        <option value="0000" name="Mode 0">description of mode 0</option>
        <option value="0001" name="Mode 1">description of mode 1</option>
        <option value="0010" name="Unknown">unknown (found in aaa.nif file)</option>
        <option value="1000" name="Mode A">description of mode A</option>
        <option value="1001" name="Mode B">GL_ONE_MINUS_DST_ALPHA</option>
    </bitgroup>
    <option value="7" name="Enable God">enable God mode.</option>
    <bitgroup value="8 9" name="God Mode">god modes description
        <option value="00" name="God One">GOD_1</option>
        <option value="01" name="God Two">GOD_2</option>
        <option value="10" name="God Three">GOD_3</option>
        <option value="11" name="God Zeus">GOD_4</option>
    </bitgroup>
    <option value="13" name="Enable Debug">show debug information</option>
    <option value="14" name="Enable Collision Geometry in Exterior Cells">draw collision geometry</option>
    <option value="15" name="Enable Wireframe">enable wireframe mode.</option>
</bitflags>

would be good if Nifskope could render:

on one line (i.e. in Block Details) in same way as in recent nifskope, i.e. text containing names of all set values separated by |. With added "flag" icon on left. See example of bitflags on line.
after left-click on flag icon Nifskope would open standalone window very similar to recent window of hard coded Alpha Property Flags. So, in this window:
- any standalone option element would be rendered on standalone line with "tickable" box and text label to the right
- any bitgroup element would cause render of dropdown menu (containing list of options within bitgroup)
- See example of bitflags window.

Having universal element for any Flags would make possible to get rid of very unconfortable ticking/unticking of bitflags in recent nifskope which render dropdown menu right from the line in Block Details.

Also editing of enum element in nifskope would be better in standalone window using radio buttons for options - see example.

@neomonkeus @ttl269 Just some initial thoughts, haven't thought about it too much yet:

<bitgroup bitstart="1" bitend="4" name="XXX">

I think that "start" and "width" would be better:

<bitgroup start="5" width="4" name="Destination Blend Mode">

You're saying explicitly that the bit width is 4 that way. Instead of the potentially confusing start=5, end=8 ... You don't really to list the end value. In this case it's immediately followed by an option with value=9. So it's a little easier to tell you've added up the bitfield columns correctly since start 5 + width 4 = 9 (the next available value).

Secondly, is having the bitgroup values in binary necessary? Could just be:

    <bitgroup start="5" width="4" name="Destination Blend Mode">destination blend mode
        <option value="0" name="One">GL_ONE</option>
        <option value="1" name="Zero">GL_ZERO</option>
        <option value="2" name="Src Color">GL_SRC_COLOR</option>
        <option value="3" name="Inv Src Color">GL_ONE_MINUS_SRC_COLOR</option>
        <option value="4" name="Dst Color">GL_DST_COLOR</option>
        <option value="5" name="Inv Dst Color">GL_ONE_MINUS_DST_COLOR</option>
        <option value="6" name="Src Alpha">GL_SRC_ALPHA</option>
        <option value="7" name="Inv Src Alpha">GL_ONE_MINUS_SRC_ALPHA</option>
        <option value="8" name="Dst Alpha">GL_DST_ALPHA</option>
        <option value="9" name="Inv Dst Alpha">GL_ONE_MINUS_DST_ALPHA</option>
        <option value="10" name="Src Alpha Saturate">GL_SRC_ALPHA_SATURATE</option>
    </bitgroup>

...keeps the "value" field in line with other usages which are (mostly) decimal.

Though this brings up a third issue. Should the "size" of the bitgroup also be defined or should the options just be counted by the parser? For example there are only 11 options for "Destination Blend Mode" even though there are 16 slots available. So, then it could also be:

<bitgroup start="5" width="4" size="11" name="Destination Blend Mode">

Though maybe a more specific word can be used other than "size". "Count" may be a better term.

@jonwd7

I think that "start" and "width" would be better. You are right, this is much better.
Secondly, is having the bitgroup values in binary necessary? I think that having binary format is self explanatory and better for person working with xml file (for parser it is maybe more complicated). Decimal values here could be little confusing because they actualy don't represent any real value, they represent only internal value of that group of bits (they are actually bit shifted to right by value in start attribute). But if using decimal value here would be much easier for parsing, I have no problem with decimal.
Should the "size" of the bitgroup also be defined or should the options just be counted by the parser? I think that it should be on parser to go through bitgroup element. I don't see any reason to have size attribute. Additionally, the person working with xml will not have to take care about number of used slots.

@ttl269, @jonwd7 - good to see this has been given some serious consideration. Just to input on the general use case as I have to take it all in.

The main issue which I think we still that needs to be address is the addressing of bit locations. I think @jonwd7 makes a good point about what system we store these as it will increase parser complexity as in the above when you switch levels.

<bitflags name="SomeFlags" storage="ushort">
    Flags flags flags.
    <option value="0" name="Enable XXX">decription of XXX enabled</option>
    //no value="1"
    <option value="2" name="Enable YYY">decription of YYY enabled</option>
    <bitgroup start="5" width="4" name="Destination Blend Mode">destination blend mode
        <offset offset="0" name="One">GL_ONE</option>
        <offset offset="1" name="Zero">GL_ZERO</option>
        <offset offset="2" name="Src Color">GL_SRC_COLOR</option>
    </bitgroup>

One thing I have noticed is that the ConsistencyType has binary option values. I am not sure internally how these are converted but this has confused me previously.

<enum name="ConsistencyType" storage="ushort">
Used by NiGeometryData to control the volatility of the mesh.  While they appear to be flags they behave as an enum.
    <option value="0x0000" name="CT_MUTABLE">Mutable Mesh</option>
    <option value="0x4000" name="CT_STATIC">Static Mesh</option>
    <option value="0x8000" name="CT_VOLATILE">Volatile Mesh</option>
</enum>

The other concern is there any flags which are conditional on the state of other. Something like

<bitflags name="SomeFlags" storage="ushort">
    Flags flags flags.
    <option value="0" name="Enable XXX">decription of XXX enabled</option>
    <option value="2" name="Enable Collision">Enables Collision system</option>
    <bitgroup start="5" width="4" name="Collision System" cond="Enable Collision == 1">destination blend mode
        <offset value="0" name="Basic">Basic</option>
        <offset value="1" name="Spherical">Spherical</option>
        <offset value="2" name="Full">Full Havok</option>
    </bitgroup>

@jonwd7 @neomonkeus Good idea to use name offset for inner elements of bitgroup. If it helps to make parser simplier. So, the tag value in offset would represent bit offset from base bit position defined by tag start in parent bitgroup element. (btw I think you wanted to use value tags instead of offset in lines with offset 1 and 2 in first example).

The 0x prefix is used for hexadecimal numbers, not binary. As I know, the prefix for binary is 0b but it don't work in nifskope.

I think that conditionals for bitgroup are not necessary. If any bit enables/disables any other bit/bitgroup, it still don't change purpose of this dependent bit/bitgroup. I don't believe that exists any flag bits with variable function depending on other bit (within same element).

Yeah, they were mainly some ideas I threw out as possible things to consider as part of the overall context as we were not using a totally real example.

it would be still good to consider these things. Parser can always be updated with new features, but when updating the domain description, its important to think about these situations which even if they never occur, its good to consider at least the possibility.

Not sure about offset for the 0 & 1 but that can be changed too. There are a lot of general name choices I can suggest better name for. Still focusing on the current situation, I would suggest if we were going all out, then I would propose the following:

<bitflags name="SomeFlags" storage="ushort">
    Flags flags flags.
    <bit offset="0" name="Enable XXX">decription of XXX enabled</option>
    <bit offset="2" name="Enable Collision">Enables Collision system</option>
    <bitgroup start="3" width="4" name="Collision System Type" cond="Enable Collision == 1">destination blend mode
        <bit offset="0" name="Basic">Basic</option>
        <bit offset="1" name="Spherical">Spherical</option>
        <bit offset="2" name="Full">Full Havok</option>
    </bitgroup>

The reason I did not propose this is it would have much larger impact on all parsers. That said, as I always say, the format description should is care if there is a large changes, just ease of xml usability.

Speaking of which @jonwd7, must sit down and talk about removing NifSkope specific data from the format.

I detail the removal of 'Flags' in #76, so read up on it there. But I will at least elaborate on the decided upon layout for the new tag:

<bitfield name="AlphaFlags" storage="ushort">
    Flags for NiAlphaProperty
    <member width="1" pos="0" mask="0x0001" name="Alpha Blend" type="bool" />
    <member width="4" pos="1" mask="0x001E" name="Source Blend Mode" type="AlphaFunction" default="SRC_ALPHA" />
    <member width="4" pos="5" mask="0x01E0" name="Destination Blend Mode" type="AlphaFunction" default="INV_SRC_ALPHA" />
    <member width="1" pos="9" mask="0x0200" name="Alpha Test" type="bool" default="1" />
    <member width="3" pos="10" mask="0x1C00" name="Test Func" type="TestFunction" default="TEST_GREATER" />
    <member width="1" pos="13" mask="0x2000" name="No Sorter" type="bool" />
</bitfield>

The attributes:

width the bit width of the member
pos the starting position of the member (zero-indexed)
mask the value to isolate the bits for this member, before right-shifting it by its position to retrieve the value
- GET: (flags & mask) >> pos will retrieve the value, flags being the entire ushort value for the bitfield.
- SET: flags = (flags & ~mask) | (val << pos) will set the value, val being the value of the member before shifting.
type on each 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.

For C++, this would look like the following

struct AlphaFlags
{
    // Cutting out AlphaFunction and TestFunction enums...

    using ushort = unsigned short;
    union {
        ushort value;
        struct {
            ushort alphaBlend : 1;
            ushort sourceBlendMode : 4;
            ushort destinationBlendMode : 4;
            ushort alphaTest : 1;
            ushort testFunc : 3;
            ushort noSorter : 1;
        } bits;
    };
    // OR just simply
    // ushort value;

    ushort GetValue() const
    {
        return value;
    }

    // If using `bits` struct, mask/pos are actually unneeded
    bool GetAlphaBlend() const
    {
        // OR return (bool)(value & 0x0001);
        return (bool)bits.alphaBlend;
    }
    AlphaFunction GetSourceBlendMode() const
    {
        // OR return (AlphaFunction)((value & 0x001E) >> 1);
        return (AlphaFunction)bits.sourceBlendMode;
    }
    AlphaFunction GetDestinationBlendMode() const
    {
        // OR return (AlphaFunction)((value & 0x01E0) >> 5);
        return (AlphaFunction)bits.destinationBlendMode;
    }
    bool GetAlphaTest() const
    {
        // OR return (bool)((value & 0x0200) >> 9);
        return (bool)bits.alphaTest;
    }
    TestFunction GetTestFunc() const
    {
        // OR return (TestFunction)((value & 0x1C00) >> 10);
        return (TestFunction)bits.testFunc;
    }
    bool GetNoSorter() const
    {
        // OR return (bool)((value & 0x2000) >> 13);
        return (bool)bits.noSorter;
    }

    // Not all setters shown for example
    void SetAlphaBlend(bool val)
    {
        // or value = (value & ~0x0001) | (ushort)val;
        bits.alphaBlend = (ushort)val;
    }
    void SetSourceBlendMode(AlphaFunction val)
    {
        // or value = (value & ~0x001E) | ((ushort)val << 1);
        bits.sourceBlendMode = (ushort)val;
    }
    void SetDestinationBlendMode(AlphaFunction val)
    {
        // or value = (value & ~0x01E0) | ((ushort)val << 5);
        bits.destinationBlendMode = (ushort)val;
    }
};

Other languages could get by with just storing the single ushort value and having the Get/Set functions with the mask/pos.

niftools / nifxml

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