OpenNaja / cobra-tools

A suite of GUI tools for extracting and modifying OVL and OVS archives, as well as editing the associated in-house file formats. Also includes a model plugin for Blender. For downloads, guides, and FAQs visit:
https://opennaja.github.io/cobra-tools/
GNU General Public License v3.0
98 stars 28 forks source link

Common struct class, _generate_attribute_list function and minor bug fixes for codegen. #220

Closed Candoran2 closed 2 years ago

Candoran2 commented 2 years ago

Summary of changes

Additions/changes:

  1. All structs inherit from the StructBase class.
  2. Removed to_stream and from_stream functions from struct classes as an aside to this.
  3. Addition of _get_filtered_attribute_list function to all structs.
  4. Initialization of fields to 0 instead of full objects before they are overwritten by defaults or file reading.
  5. Apply naming conventions to enum keys when they have whitespace or lowercase letters.
  6. Added check for template not being None in case of generic=true attribute on the xml element.
  7. Removed standard attribute setting from compounds with parents, under the assumption that this already happens in the parent class.

Bug fixes:

  1. Fixed version number not printing if it was 0.
  2. Changed import of versions module to work properly.
  3. Fixed instances where consecutive identical conditions were not grouped together, causing unnecessary checking of conditions that were already evaluated.
  4. Changed fmt_member function to be imported from generated instead of source, like all other code.
  5. Prevent trying to import the template class.

Detailed changes

  1. Like the bitfield and enum classes, which have a common base class for all classes of the same tag, structs now all inherit from the StructBase class. This allows for easier checking if something is a struct, as well as for simple changes of behaviour/addition of convenience functions without needing to change codegen, should it be required. This also means ContextReference no longer needs to be added/imported to the generated classes, since StructBase already contains it.

  2. Since to_stream and from_stream code was not dependent on the class, I've moved them to StructBase and removed them from codegen.

  3. The old pyffi had the _get_filtered_attribute_list function for (most?/all?) classes. It allowed looping over active attributes (i.e. ones whose conditions evaluated to true ). Attributes contained information about the field name, type, as well as the various attributes in the corresponding XML element. Because this is a useful feature that can have many functions, I've added a similar function to all structs to allow looping over the fields and provide similar information. The new function provides, for every field that is active, the following information in a tuple: (name, type, (parameters)) There are two distinct cases:

    • The field is not an array. In this case, what is returned will be (name, field_type, (arg, template)).
    • The field is an array, in which case the returned values will be (name, Array, (shape, field_type, arg, template)). In this way, a field with an array can almost be considered to have a type of Array with extra parameters. It should be noted that in order for this to work properly with recursion for multi-dimensional arrays, the Array class still needs to be modified to be nested Arrays instead of nested lists, or we need to work on the assumption that Arrays are at most two-dimensional (which is currently the case, but needn't necessarily be in the future). In theory this single function could be used to remove many of the current functions that are part of codegen, like field reading/writing and printing, although it doesn't have to, of course.
  4. Fields are initialized to 0, instead of full-blown objects, in the init function. This is because either set_default=True, which will fill all the fields with default values anyways and overwrite those objects, or set_default=false and they are overwritten by the values as read from file. This way, unnecessary overhead is removed. The reason the fields are still initialized at 0 rather than not at all is because some fields are used in conditions and must therefore always exist in some form or another, at least for nif.xml.

  5. Apply naming conventions to enum keys. Some enum keys in nif.xml have spaces in them, meaning that they aren't suitable for access to the enum in the usual way. To counteract this, a formatting function is applied to them. However, this would also change a number of existing enum keys. Especially texture format keys become less legible when that format function is applied, because it adds separation between numbers and letters. As a compromise, the formatting function is only allowed when the enum key has whitespace or lowercase letters.

  6. Some XML elements for structs have generic="true". This means that the template must be specified when they are used. This could be accomplished by switching around the arg and template parameters in the function signatures and then removing the template default value. However, because this would involve a large rewrite, instead an extra check that template is not None (the default value) was added. The only substantial functional difference between this option and the former is that you can not get around the error by specifying template=None, but I don't think there is a current use case where you want template=None.

  7. The attributes name, arg, template, io_size and io_start setting in the __init__ function of compounds was always the same, so if a class had a parent class, then it already happened there. The codegen now takes this into account, only adding the setting of these attributes in the init if the compound has no parent class.

  8. If the version was not parseable, the value would be set to None. This was used to decide whether it was printed or not. However, it used if self.value, so a value of 0 would also not print. This is now fixed to explicitly check for None.

  9. Import of a format's version module in generated code used the wrong path. This is now fixed.

  10. Because condition was always assigned the new condition in the compound code, and the new condition became empty if the newer conditions were the same as the previous one, the Union after that though there was no previous condition. This is now fixed by only assigning condition to the new condition if there is a change (new condition is not empty, or it is empty because there are no conditions on this field).

  11. As it says. I presume this was a simple oversight.

  12. Template is not a class or module, and therefore came be imported. Previously template became self.template in the attribute (and things starting with self. aren't imported), but because of changes in the code this no longer happens. template has now been added to the "classes" that don't need importing.