Argument Relative Assignment Fix

Reporter: RobertBColton  |  Status: closed  |  Last Modified: August 06, 2020, 07:50:17 AM

This fixes an issue with emake's drag and drop conversion that was causing argument_relative not to be assigned correctly. It specifically was making TKG's Key to Success game have collision bugs when run from emake. The problem was that JDI has a setting to treat = assignment as == comparison like old GM, which interferes with the behavior of the converted drag and drop code. LGM plugin solved this problem by using the := assignment operator, but everybody on Discord pushed me to just make it its own separate statement so it's more human-readable. With this change, TKG's Key to Success works perfectly from the command line.
codecov[bot]  
>Codecov Report

Merging #1991 into master will increase coverage by 0.64%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1991      +/-   ##
==========================================
+ Coverage   30.95%   31.60%   +0.64%     
==========================================
  Files         197      197              
  Lines       19115    19497     +382     
==========================================
+ Hits         5918     6162     +244     
- Misses      13197    13335     +138     
Impacted Files Coverage Δ
ENIGMAsystem/SHELL/Platforms/xlib/XLIBwindow.cpp 49.16% <0.00%> (-2.50%) ⬇️
...NIGMAsystem/SHELL/Universal_System/terminal_io.cpp 4.34% <0.00%> (-1.21%) ⬇️
ENIGMAsystem/SHELL/Platforms/SDL/Window.cpp 58.25% <0.00%> (-0.57%) ⬇️
ENIGMAsystem/SHELL/Platforms/SDL/Clipboard.cpp 0.00% <0.00%> (ø)
ENIGMAsystem/SHELL/Platforms/General/PFwindow.cpp 43.34% <0.00%> (+2.04%) ⬆️
.../Universal_System/Object_Tiers/graphics_object.cpp 42.50% <0.00%> (+2.50%) ⬆️
...system/SHELL/Graphics_Systems/General/GSscreen.cpp 65.39% <0.00%> (+2.58%) ⬆️
ENIGMAsystem/SHELL/Platforms/xlib/XLIBmain.cpp 39.70% <0.00%> (+2.66%) ⬆️
...GMAsystem/SHELL/Platforms/xlib/XDisplayGetters.cpp 96.77% <0.00%> (+4.18%) ⬆️
ENIGMAsystem/SHELL/Universal_System/fileio.cpp 89.88% <0.00%> (+6.81%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b0375a3...95209cf. Read the comment docs.

JoshDreamland  

This also has the possibility to introduce regressions. Does Key to Success feature a "Question" action that is relative? You might have broken them.

  1. Do we have tests for any of this? Actual tests, not just running one game.
  2. Does this change (or a similar change) also need to be made to LGM's Action generator?
RobertBColton  

We don't have drag and drop tests yet, no, but they are needed. This change does not need to be made to LGM's action generator, no, because it uses the := assignment operator from GML, because the author knew well enough that JDI converts = to == in parentheses. That was the mistake I made, I didn't understand why := was being used in lgmplugin, and thought it looked silly.

I don't know about your first question, but I did test Key to Success against this and this makes it work the same as LGM.

JoshDreamland  

IIRC, the result of an action_if_* function is used directly in an if-statement when emitting code. That's why we inlined the assignment. I am asking you if you've tested with any of those. I doubt Sam happens to be using a DND if with argument_relative.
RobertBColton  

I still don't know the answer to that, but I do believe it was specifically a question action broken in his game. Also, while lgmplugin used the result inline, emake uses the __if__ fix for "applies to" fix like GMS. The lgmplugin actually still needs that fix, however I've found issues with it IIRC which is why it's not yet merged into the plugin.
fundies  

@RobertBColton when you merge this and #1989? I would like emake to work proper pls
RobertBColton  

Soon
RobertBColton  

@JoshDreamland I had to double check what you were saying here and I couldn't find it to be the case. I don't think I originally understood you but I do now in that you were concerned any of the action_if question actions take argument_relative as a first parameter instead of using the global argument_relative variable. I could not find a single action if function in ENIGMA that has relative as its first parameter, so that could have just been a mistake in the original LGM plugin I copied over.

If you go use ActionMaker to open the GM action libraries, you can see several questions use argument_relative but don't have it as a parameter. I don't even see an option to have it as a parameter.
https://robertbcolton.github.io/ActionMaker/
Action Maker Question Relative

Also if we inspect the conversion code some more, we see the function name/call was inside the parentheses after where the assignment I removed was.

code += action.function_name();

So really I think you just misread and this should be fine. Although, we could/should further clean that up maybe to not even use parens there since we use __if__ now. I agree it needs tests because I'm almost certain somebody is going to break it.

code += "__if__ = ";

Please sign in to post comments, or you can view this issue on GitHub.