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

[Spec] Block contracts #55

Open
hexabits opened this issue Apr 13, 2016 · 1 comment
Open

[Spec] Block contracts #55

hexabits opened this issue Apr 13, 2016 · 1 comment

Comments

@hexabits
Copy link
Member

hexabits commented Apr 13, 2016

A contract is a formal set of obligations which a data structure must follow. Generally it is a precondition, a postcondition, or an invariant (an assertion which must stay true). You can read up on it here.

For our uses a contract would be something that must be true for a block, evaluated at parse time. Contracts can provide additional details about a block that otherwise wouldn't be defined (i.e. it's a form of documentation) and can be used for validation as well as performance optimizations.

Reasoning

For Fallout 4, a vertex is now a compound with 15 conditional rows. NifSkope evaluates the conditions per row for each vertex which is unnecessary because the conditions are passed down from the BSTriShape, thus every row will have the same conditions on each vertex. So, to parse this compound at any reasonable speed I inserted an attribute on my shipped nif.xml which tells NifSkope the cond are all external (i.e. ARG) and to optimize this compound.

With a contract on the BSVertexData compound I was able to cache the conditions for subsequent vertices and speed up reading by over 12x.

Suggested Contracts

External Conditions externalcond

  • The <add> in this block can only have cond attributes which use a passed in ARG.
  • Error Checking: The cond attribute cannot contain the names of any <add> before it. (This is naive and would fail if a row name were "ARG" but easier than actual expression parsing.)

Potential Contracts

No Conditions nocond

  • The <add> in this block cannot have cond attributes.
  • Error Checking: The presence of a cond attribute indicates immediate failure.

No Version Conditions novercond

  • The <add> in this block cannot have attributes related to version checking.
  • Error Checking: The presence of a vercond, ver1, ver2, etc. attribute indicates immediate failure.

Fixed Size fixed

  • The block is guaranteed to always be the same size in a file. This means it contains no variable arrays, internal or external conditions, or child compounds with these things.
  • Error Checking: The presence of a cond attribute indicates immediate failure. Additionally, referenced compounds must also be fixed, and the attributes arr1 and arr2 cannot contain a variable.

Basic basic

  • The block is fixed + nocond + novercond and additionally only contains basic types (i.e. no compounds). Examples: Color3, Vector3.
  • Error Checking: The type attribute cannot point to anything but a basic block. Additionally, run the logic for fixed, nocond, and novercond.

Implicit Contracts

Some contracts would be implicit for a block type and not need to be defined.

  • <niobject> is inherently internalcond as you cannot pass ARG into it. (Ideally the parser should make certain that the cond on an <niobject> are not using ARG as an operand.)
  • <compound> is inherently mixedcond as it can use both internal and external conditions.

There is not much use for explicitly defining internalcond or mixedcond for anything.

Proposed Syntax

Single contract

<compound name="BSVertexData" implements="externalcond">

Multiple contracts

<!-- Correct way to make lists in XML attributes is space-separated -->
<compound name="BSVertexData" implements="externalcond novercond">

Parsing

Implemented as a new attribute in this way, the XML would still be backwards compatible. In fact different parsers would never have to support this at all if they did not want to. But when it comes to authoring the XML, one would still have to write valid contracts and check on parsers who do use them.

As for error checking, each contract need only be concerned with its own rules, and interactions between contracts shall not be a concern. Take for example externalcond + nocond.

Up for discussion

  • Do we this for only compounds or do we also do it for niobjects? The answer is now unquestionably yes.
  • How we document the contracts at the top of the file.. Do they get their own XML tag like <version>? Or just a comment block?
  • Which of the "potential" contracts do we really need? Are there any contracts you could think of that would be useful for documenting or validating the data behavior?
  • Is the syntax sufficient, or would anyone care to propose alternatives?
@neomonkeus neomonkeus added this to the Version 1.0 milestone Apr 13, 2016
@neomonkeus
Copy link
Member

neomonkeus commented Apr 29, 2016

Do we this for only compounds or do we also do it for niobjects?

  • Not sure, we can expand if there is a use case at a later point

How we document the contracts at the top of the file.. Do they get their own XML tag like ? Or just a comment block?

  • Explicit is always better, but not sure if we need their own tags, think that is a style choice, so open on this part.

Which of the "potential" contracts do we really need? Are there any contracts you could think of that would be useful for documenting or validating the data behaviour?

  • I think the above is probably the initial set, we can expand as needed. We can validate the implicit ones too through some form of format validator. Probably need to build and store an actual schema.

Is the syntax sufficient, or would anyone care to propose alternatives? If the syntax is fine, I will submit a PR for externalcond usage ASAP, as it's needed to merge nifskope dev/2.0 to develop.

  • Think so

@hexabits hexabits changed the title Block contracts [Spec] Block contracts May 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants