Grimrukh / soulstruct

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

negating some @no_negate tests may temporarily checkout every condition and fail to compile #14

Closed LukeLavan closed 2 years ago

LukeLavan commented 3 years ago

Consider the following event: eventthatshouldntcauseissues The first Await works properly, since no negation is attempted on the PlayerInCovenant test. However, PlayerInCovenant is a @no_skip_or_negate_or_return test, so the next Await which tries to negate the test must check out a temporary condition to do so after attempting to compile the negated test throws a NoNegateError.

This happens in evs.py during the relevant call to self._compile_condition here. So far in this event, no conditions have been used, and so the call to self._check_out_temp returns -7 for the condition to use - so far so good.

The skip_negate boolean is initialized to True since we're not trying to skip lines but we are trying to negate. The logic string is initialized to 'False' and we determine we want to use the instr SkipLinesIfConditionFalse to simulate the negation.

Finally, we attempt to compile this temporary condition. The node passed in is node_to_recur which contains the original ast.Call with the func attribute of the ast.Name PlayerInCovenant. This is not the same as the current node since it was replaced further up in self._compile_condition here.

Notice that the arguments to this recursive call to self._compile_condition are exactly the same as where we started - this recursive call will try to compile the PlayerInCovenant test with negate=True. The only difference is that the -7 condition has already been checked out - so the +7 condition gets checked out this time.

This continues until all possible conditions have been temporarily checked out, at which time the self._check_out_TEMP call will fail and error here.

This part of the code works as intended when negate=False or if skip_lines > 0. However, because the Await() evaluates tests with skip_lines=0 and negate=True, the arguments to the recursive call to self._compile_condition end up being the same as the original call.

I'm not sure what the intended control path is for PlayerInCovenant. For reference, the correct syntax for the event with PlayerInCovenant is as follows:

correctsyntax

LukeLavan commented 2 years ago

It's been a year since I opened this issue. Since then it looks like the emevd stuff has drastically changed. I'm not sure if this issue is still present or relevant anymore, so I'm going to go ahead and close the issue and associated PR.