niftools / nifxml

A repository for the nif.xml file, which contains the nif file format description.
http://www.niftools.org
GNU General Public License v3.0
37 stars 43 forks source link

Enum usability / definition issues #63

Closed hexabits closed 7 years ago

hexabits commented 7 years ago

There have been numerous issues and inter-project fights over the values of the name attribute in <option>. There is also a usability issue in that it is impossible to edit enum or bitflags options by hand and know that you haven't created a non-unique name. niflib needs unique option names for the codegen from nif.xml (See: https://github.com/niftools/nifdocsys/blob/master/gen_niflib.py#L412)

There is an additional problem in that the nice neat names we give to option tags for the UI become much uglier when adding the unique prefix for the enum (See #62).

I propose:

Add a prefix attribute to <enum> and <bitflags>

The prefix attribute would be prepended to each name, preventing collisions. For applications where the uniqueness doesn't matter and where the name are used for UI, this keeps the name clean but allows codegen to enforce uniqueness via the prefix.

Example

    <enum name="EffectShaderControlledVariable" storage="uint" prefix="ESCV">
        An unsigned 32-bit integer, describing which float variable in BSEffectShaderProperty to animate.
        <option value="0" name="EmissiveMultiple">EmissiveMultiple.</option>
        <option value="1" name="Falloff Start Angle">Falloff Start Angle (degrees).</option>
        <option value="2" name="Falloff Stop Angle">Falloff Stop Angle (degrees).</option>
        <option value="3" name="Falloff Start Opacity">Falloff Start Opacity.</option>
        <option value="4" name="Falloff Stop Opacity">Falloff Stop Opacity.</option>
        <option value="5" name="Alpha Transparency">Alpha Transparency (Emissive alpha?).</option>
        <option value="6" name="U Offset">U Offset.</option>
        <option value="7" name="U Scale">U Scale.</option>
        <option value="8" name="V Offset">V Offset.</option>
        <option value="9" name="V Scale">V Scale.</option>
    </enum>

    <enum name="LightingShaderControlledVariable" storage="uint" prefix="LSCV">
        An unsigned 32-bit integer, describing which float variable in BSLightingShaderProperty to animate.
        <option value="0" name="Refraction Strength">The amount of distortion.</option>
        <option value="8" name="Environment Map Scale">Environment Map Scale.</option>
        <option value="9" name="Glossiness">Glossiness.</option>
        <option value="10" name="Specular Strength">Specular Strength.</option>
        <option value="11" name="Emissive Multiple">Emissive Multiple.</option>
        <option value="12" name="Alpha">Alpha.</option>
        <option value="20" name="U Offset">U Offset.</option>
        <option value="21" name="U Scale">U Scale.</option>
        <option value="22" name="V Offset">V Offset.</option>
        <option value="23" name="V Scale">V Scale.</option>
    </enum>

It's also the easiest way to ensure uniqueness when editing by hand as you no longer need to check the entire file for the the same name string, only assure that it is unique inside that enum/bitflag.

Adoption

For this to work, gen_niflib.py would have to be updated to account for the new prefix attribute. It would have to prepend the prefix to each name attribute on write.

hexabits commented 7 years ago

@neomonkeus So do you understand nifdocsys enough to look into how to go about changing gen_niflib.py to prepend this prefix? I looked at it very briefly and I can see that it writes them starting here https://github.com/niftools/nifdocsys/blob/master/gen_niflib.py#L412 but I don't see yet where it sanitizes the names to remove spaces and such.

hexabits commented 7 years ago

@aperture-it Since you were able to so easily change genniflib.py for uniquename do you think you could look into adding this prefix attribute for <enum> and <bitflags>? Regarding 9b45d8a4bf0fb5818ee15851776953e23e153524 ... all you would have to do was add `prefix="ESCV"andprefix="LSCV_"to theinstead of changing every

It also has the benefit of foolproofing name changes or additions so that future nif.xml edits do not break niflib.

aperture-it commented 7 years ago

Yes, adding prefixes and suffixes is not complex. In fact, this is not done in gen_niflib.py, but in nifxml.py.

hexabits commented 7 years ago

Did you mean to post this in #64? If you are proposing an alternative to uniquename we should discuss it there.

hexabits commented 7 years ago

I have removed the _ from the prefix in my proposed syntax to be consistent with the suffix proposal in #64. The underscore can be added by the parser.