Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for version, bitflags, bitfield and default tags. #129

Merged
merged 23 commits into from
Sep 30, 2021

Conversation

Candoran2
Copy link
Contributor

@Candoran2 Candoran2 commented Aug 3, 2021

Changes

  1. Bitflags are now processed, treated like bitfields where every field is width 1 and a bool.
  2. Arr1 and Arr2 now can also use the alternative attribute names length and width, respectively, to match with the newer nifxml format.
  3. The text in version tags can now handle more than 1 game.
  4. The {{...}} double curly brackets for default versions for a game are now used: set_game checks for them first, and returns after setting it to that version.
  5. All possible games (+ UNKNOWN_GAME) are now put into an Enum, and the get_game and set_game functions use this enum.
  6. Check from the ver2 attribute is now <= instead of <, in line with the intended nifxml function of that attribute.
  7. OnlyT and ExcludeT now work like they're supposed to.
  8. The non-optional 'context' argument was added to generated compounds/structs and NiObjects.
  9. The formatting of the version id (for use in set/check function) is separated out into its own function.
  10. The default tag is now parsed and handled.
  11. The default encoding for the generated files is now utf-8 instead of being environment-dependent.
  12. Bitfields/bitflags are now initialized as such, instead of ints.
  13. Imports did not yet import anything in a field's length attribute (alias for arr1 used by newer nif.xml). Now does it correctly.
  14. Removed unused functions from codegen.py.
  15. Bitfield no longer inherits int(), and implements all numeric-type emulation functions.
  16. Modules now take precedence over class tag in generated directory structure and apply to all classes.
  17. __init__.py added to all generated subfolders
  18. Changed expression evaluation on cond and vercond attributes to be in line with nifxmlspec issue 73.
  19. The name_filter argument was dropped from the Expression class, as well as the eval and map_ methods.

Detailed changes

  1. Bitflags were already processed by the bitfield code, but it errored because there was no code to handle the absence of width, pos and type. These are now added/processed when the bit attribute is present (i.e. bitflags). The error message itself also contained an error (field.name) that is now fixed (field.attrib['name']).
  2. The function get_attr_with_backups was added since this pattern was used a lot (multiple possible attribute names for the same function).
  3. The version tag text is split by , as in nifxml. get_game() now always returns a list, while set_game() checks whether the game is in the set for that specific version. The version tag is excluded from the apply_conventions() function so that this is easier processed (and the apply_conventions function had no other effect on version tags anyway). This also means that get_game returns a list, rather than a string.
  4. set_game first checks for the game in versions which had a default (and only in their defaults), and only then goes to check for non-default game inputs. It also returns as soon as it has found a version with matching game, rather than continuing after that.
  5. The games are put into an Enum per format, whereas it was previously a list. The get_game and set_game functions also use this enum. This means that for set_game(), you must access the enum to pass that option.
  6. Relatively self-explanatory.
  7. The classes referenced by OnlyT and ExcludeT are added to the imports, and the check for excludeT and onlyT now uses isinstance() instead of just checking that the referenced class is not None.
  8. The newly added context variable allows for field existence/values on initialization to depend on the version of the file they're created in (which happens in a number of cases in the xml, even fields that change type depending on version).
  9. Separating the version ID formatting is used for access by other classes that might want to use it (like the versions attribute checking, which was necessary for the default tag).
  10. The default tag is now handled. Everything is done except the import of the version checking functions. In order to do this, I've separated out the code for getting the conditions and the default value for a field and re-applied them on the default tags, if present. In the xml later default tags override previous ones. To achieve the same effect, the codegen checks them in reverse order and applies the first one it finds (should be more efficient than setting the default value and letting it override). There is also some extra checking to make sure there are no default tags which have the same conditions.
  11. The encoding type is now hardcoded on the XmlParser object. Every file write operation for the generated code uses that encoding (rather than the default, which is environment-dependent). It was erroring for me on the full nif.xml due to some unicode characters (like ) in the xml. This was fixed by setting the encoding to utf-8. While a rare case, it is theoretically possible to have a non-utf8 encoding for the xml (e.g. utf-16) that will still error when trying to write. If that needs to be addressed, we could take the most permissive encoding between utf-8 and the xml encoding. However, to access the xml encoding, we would have to switch xml parsers from etree to lxml. I've tried it out, and because they are structurally similar it takes very little effort. However, it is not a standard library.
  12. Relatively self-explanatory.
  13. Relatively self-explanatory.
  14. Best I could tell, both by print statements within the functions and checking for errors on removal when running codegen.py, the functions collect_types(), write_file() and get_names() were unused. As such, they were removed.
  15. The bitfield class inherited int. However, because int is immutable, the value used when right-adding (or other arithmetic operations) always used the initialized value (I think). Moreover, the previous implemented numeric emulation functions (such as __add__) edited the value of the bitfield in-place. It has now been changed such that only the actual assignment functions change the bitfield value. The rest of the arithmetic functions simply run the same function on the _value attribute of the object and return the result. The implemented functions are taken from here: https://docs.python.org/3/reference/datamodel.html#emulating-numeric-types
  16. Module paths previously used to only apply to niobjects, and were implemented via the directory structure formatname/niobject/modulename. However, the newer version of nif.xml also applies modules to structs and some enums and bitflags. Therefore, the generated directory structure is now formatname/modulename/niobject, formatname/modulename/struct etc. Moreover, an __init__.py file is added to every module path which sets the attributes __depends__, __priority__ and __custom__ in the module, taken from the attributes depends, priority and custom in the xml. While this information is not necessarily used by anything at the moment, it means they can be accessed.
  17. Without __init__.py in the subfolders, running (for example) TimeControllerFlags gives an error when trying to reload the modules. To solve this, an empty __init__.py is added to every (sub)folder in the generated code, except those starting with a double underscore (to prevent them from being added in __pycache__ folders).
  18. The nifxml spec specifies that the vercond attribute should only refer to global variables, while the cond attribute should only refer to local variables. In the old codegen, there was no difference between the evaluation of cond and vercond, and whether a variable was local or global was determined whether it had 'version' in the name. This leads to problems when evaluating the nif.xml, because there is a global variable which does not have this (BS Header\BS Version). Moreover, it also misclassifies any local variable with 'version' in the name. Hence the update to be in line with the spec, which neatly separates the two.
  19. The name_filter argument is only necessary when needing a customisable naming convention. However, all expressions take it from attributes, be that on self or on the context, hence there is no need for this argument. The eval method is used to evaluate an Expression object to an int value, but Expression objects are not used in the generated code, making the eval method obsolete. The map methodis similar. Though it also has potential uses in string manipulation, all necessary ones are already covered in the _parse method.

@HENDRIX-ZT2
Copy link
Contributor

Oh, this sounds great! Unfortunately I'm moving and renovating so don't have too much time to review!

@HENDRIX-ZT2
Copy link
Contributor

One thing of note, IIRC the xml spec for lists recommends using a blank space as separator rather than a comma. Imo nif xml should be adjusted to adhere to that.

@Candoran2
Copy link
Contributor Author

One thing of note, IIRC the xml spec for lists recommends using a blank space as separator rather than a comma. Imo nif xml should be adjusted to adhere to that.

That is the case for most (I think all) attribute values in the nif xml. In this case that wouldn't be possible because the game names can include spaces (e.g. Skyrim Special Edition).
A possible 'solution' would be to create elements within the version elements with a new tag, game, e.g.

    <version id="V3_3_0_13" num="3.3.0.13">{{Munch's Oddysee}}, Oblivion</version>

would become

    <version id="V3_3_0_13" num="3.3.0.13">
        <game name="Munch's Oddysee" default="true" />
        <game name="Oblivion">
    </version>

Although as long as you're creating a new tag, you could also do this instead:

    <version id="V3_3_0_13" num="3.3.0.13" />


    <game ID="MUNCH_S_ODDYSEE" default="V3_3_0_13">Munch's Oddysee</game>
    <game ID="OBLIVION" default="V20_0_0_5_OBL" nondefault="V3_3_0_13 V10_0_1_0 V10_0_1_2 V10_1_0_101 V10_1_0_106 V10_2_0_0__10">Oblivion</game>

Or something similar, where the game is a separate tag that contains information about which versions it appears in, and not the other way around. I don't know which is most useful. If you still want to have the version to contain information about the games, but can't have spaces in attribute names, you could also do something like this:

    <game id="MUNCH_S_ODDYSEE">Munch's Oddysee</game>
    <game id="OBLIVION">Oblivion</game>

    <version id="V3_3_0_13" num="3.3.0.13">
        <vergame game="MUNCH_S_ODDYSEE" default="true" />
        <vergame game="OBLIVION" />
    </version>

One upside to having separate elements within the version elements (aside from conforming to the spec) would be that you could have a description of the purpose for the different versions in the same game. Of course it does add extra complexity for a feature that, at least at the moment, is almost unused anyways.

For now I've just stuck with what was laid out in issue 69.

@HENDRIX-ZT2 HENDRIX-ZT2 self-requested a review August 25, 2021 22:31
@Candoran2 Candoran2 changed the title Updates for bitflags and version tags. Updates for version, bitflags, bitfield and default tags. Sep 16, 2021
… different names, and added 'length' and 'width' alias for 'arr1' and 'arr2', respectively.
…sion can use multiple games (separated in the xml by a ', ', and that certain versions may be the default for a particular game. It also uses an enum instead of the game name strings.
…e '10.0.1.0' are converted to their proper integer values (like they are in field attribute conditions)
… the type of the object that the field is on.
…s, so that they can be used when initializing fields.
… struct/module) and applied them to all tags. Codegen adds __init__.py file to this path where the module attributes are set.
…e 73 of nifxml spec: cond is for local variables, vercond for global variables. Updated source xmls accordingly, should not result in a change of generated code (except for nif.xml).
…pression class on Hendrix' recommendation, because they are not used.
@HENDRIX-ZT2 HENDRIX-ZT2 merged commit 2f8a7a1 into OpenNaja:master Sep 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants