search

Grimrukh / soulstruct

Python tools for inspecting and modifying FromSoft games (mainly Dark Souls 1).
145 stars 16 forks source link

allow negative numbers and global variables in simple comparisons #13

Closed LukeLavan closed 2 years ago

LukeLavan commented 3 years ago

negative numbers

When performing simple comparisons like so:

problem

compilation yields the following error:

problem_compile

This error occurs because the right value (node.comparators[0]) currently needs to be an ast.Num. However, negative numbers are parsed as an ast.UnaryOp because of the way Python handles numerical literals:

Note that numeric literals do not include a sign; a phrase like -1 is actually an expression composed of the unary operator ‘-‘ and the literal 1.

I circumvent this by using the ast.literal_eval function and packing that back into an ast.Constant. Doing so allows the compilation to succeed, and decompiling the product shows that this is handled successfully:

solution

As a sidenote, the use of ast.Num is deprecated:

Deprecated since version 3.8: Old classes ast.Num, ast.Str, ast.Bytes, ast.NameConstant and ast.Ellipsis are still available, but they will be removed in future Python releases. In the meantime, instantiating them will return an instance of a different class.

Changed in version 3.8: Class ast.Constant is now used for all constants.

and so is using the .n attribute of ast.Constants:

deprecate

so my changes also make the appropriate refactors.

global variables

Also disallowed for the right side of simple comparisons were ast.Attributes, which could be global variables, either imported or declared in the file:

import_desired

global_desired

both result in: import_error

Using my changes, both compile successfully. This works as proved by the result of decompilation:

global_desired results in global_solution

and import_desired results in import_solution

as expected, the decompiled results are equivalent.

This is accomplished by passing in the EVSParser's self.globals along with the comparison node to the _validate_comparison_node function. Let me know if this isn't desirable behavior or if there's some better way to do this!

LukeLavan commented 2 years ago

Looks like the global variable part of this PR was addressed in this commit by making _validate_comparison_node a member function, allowing access to use _parse_attributes. Negative numbers still aren't accounted for, so I updated my branch accordingly and I will leave this PR up. Note however that the permalinks point to the old version of this function - if desired I can update these links as well.

LukeLavan commented 2 years ago

looks like https://github.com/Grimrukh/soulstruct/commit/7b464b7457ff177023cbe30dcb1338fc70c93038 fixes this by passing the comparator to self._parse_nodes which contains logic to evaluate a negative number