Match failure.

Match failure.

Match failure.

Migrate compiler to a GameData structure.

Reporter: JoshDreamland  |  Status: closed  |  Last Modified: January 19, 2019, 06:34:21 PM

We're currently taking input from two sources: LateralGM (via the EnigmaStruct shared-memory model) and RadialGM (via the new Project) proto.

Passing either of these structures around in the compiler is error; the former is tailored to C-like languages which structure everything in memory manually, and the latter is tailored to the IDE tree and is unfit for quick processing.

By migrating both to a flat model that is proto-based, we can save code and do the flattening up front.

This request is not ready to be merged and is mostly for running incremental builds on the changes.

codecov[bot]  
>Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1448      +/-   ##
=========================================
+ Coverage   17.42%   17.6%   +0.18%     
=========================================
  Files         165     165              
  Lines       17127   17127              
=========================================
+ Hits         2984    3015      +31     
+ Misses      14143   14112      -31
Impacted Files Coverage Δ
...m/SHELL/Platforms/General/POSIX/POSIXdirectory.cpp 0% <0%> (ø) ⬆️
.../SHELL/Universal_System/Extensions/GTest/include.h 100% <0%> (ø) ⬆️
ENIGMAsystem/SHELL/Universal_System/mathnc.h 98.3% <0%> (+1.69%) ⬆️
.../SHELL/Universal_System/Extensions/GTest/GTest.cpp 73.68% <0%> (+31.57%) ⬆️
ENIGMAsystem/SHELL/Universal_System/random.cpp 83.78% <0%> (+64.86%) ⬆️

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 138ec8e...990f744. Read the comment docs.

EnigmaBot  

UNMATCHED: Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

ac0f410 Master Diff
Image Diff Image Diff Screen Save

EnigmaBot  

UNMATCHED: Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

f80e14e Master Diff
Image Diff Image Diff Screen Save

EnigmaBot  

UNMATCHED: Regression tests have indicated that graphical changes have been introduced. Carefully review the following image comparison for anomalies and adjust the changeset accordingly.

f83d57c Master Diff
Image Diff Image Diff Screen Save

RobertBColton  

Writing it down this time so you don't forget what I can see is broken:

The game icon is using the game info icon and I can't close the game.

Wrong Game Icon

Fonts are invisible or something so no text is drawn in any game

Invisible Fonts

Backgrounds are missing in some games

Game of Life Black Background

Room tiles don't appear but tile_add seems to work

No Room Tiles

JoshDreamland  

Again, I don't copy game settings or game information, at all, right now, and something about the background export is broken.
RobertBColton  

Ok, writing down some more things I run into as I work on this.

Extensions/Constants Missing Proto

Extensions don't have a proto just because I think I forgot to get around to them. I wasn't sure if we were planning to keep supporting GM extensions and ENIGMA extensions. There's also another problem, GMSv1.4 had extensions as resources where you could add includes using the tree. As I recall, they stopped supporting "installed" extensions that you place into the GM installation folder. LGM coming from such a time in GM's history only ever had the name and path of an extension but not all of its other metadata. I didn't know what to do, if we're going to do extensions as a resource, installed extensions, or only ENIGMA extensions. One idea I have is we could do the new GM way and support legacy "installed" extensions by pretending they are ENIGMA extensions. Really need your input on this.

GMSv1.4 Create Extension
GMSv1.4 Export Resources

There's no constants proto because they are deprecated in GMS2 and I want to deprecate them in ENIGMA too. In GMS2 you can create a script and use the #macro directive to create a global constant. I would like us to add this to ENIGMA, since people are requesting it, and then libEGM will generate a script with a #macro for each constants resource when loading an old GM project.

RobertBColton  

@JoshDreamland alright: 4553ae2
I imported the game settings from EnigmaStruct and it seems to be working now. I have a game icon, title, and buttons as well as the ability to close the game. The said, I need to clarify to you that we are planning on multiple game settings in the proto (hence why we just have a tree) and that the one that gets written is supposed to be the one in the "selected configuration" combo box.

Also, you seemed to have missed the part about the shortcuts. Me and fundies want to make the game settings allow you to remap the shortcuts to any input device. You would also be able to add new shortcuts and remap them as you wish. By default, we would use entries like "close_game" to determine what input closes the game.

Game Settings Imported

Game Settings Imported to Game Data

Game Settings Input Mapping

Game Settings Input Map

Current Config Selector

Current Config Selector

RobertBColton  

Picking up where we last left off, I made a test project with a background and noticed this branch and master are always writing it with 426619 size. So that rules out that possibility.

I then tested draw_background and it actually worked. So the whole tiles/backgrounds issue must be confined to rooms only. This led me to do a diff on IDE_EDIT_roomarrays.h where I finally found the difference to be the tile and background color and alpha.

Room Array Differences

I see this must have happened because we weren't actually defaulting the tile and background alpha and color. Since EnigmaStruct also didn't have a corresponding field, Proto2ES was not yet actually using these values, only retaining them. Regardless, the fix is simple.

/*es->rooms[i].tiles[ii].color*/0xFFFFFF << "},";

JoshDreamland  

Good find. I remember making that change consciously... I saw some commented fields that were wishful thinking on my part because LGM was so hard to coordinate additions to. I modified the code to actually use proto data now that it's available. I believe I assumed that the fields would have default values. If that's not the case, it should be. The default tile color should be white, and the default tile alpha should be opaque.

If you're avoiding that for some reason, please set those values in the appropriate GameData ctors. You may have noticed that this class functions essentially like ES2Proto.

RobertBColton  

Ok, so the thing here is that we hadn't exactly decided on where defaults should go. Back when I asked you about setting the defaults in the proto, you sort of scoffed at that in favor of "smarter" defaults. That's why we aren't setting any basic defaults (such as default tile size of 32x32) anywhere in any of the protos. So are you saying it's ok to use the protobuf default option on fields now? Can I use the defaults LGM/GM have had for some time?
JoshDreamland  

I'm glad you're keeping that in mind. Let me reiterate; it may make more sense, at this point.

A default can go in the proto if it's not specific to any one type of game or IDE. So it's fine to default the tile color to opaque white, because that's a normal tile—the default default of zero will break things while white/opaque will make things work as expected. An example of a bad default is a room color of cornflower blue, because it fits well with the RadialGM color scheme. You can justify cornflower in other ways—eg, it makes black drawings and white drawings both show up by default. The idea is, a default should serve some greater purpose than whimsically not being zero.

Perhaps it's good to apply the zero, one, infinity rule, here. We'll make extra allowances for noone, c_white. The point is, 16 is not a good default tile spacing. Blue is not a good default blend color. And 32 is not a good default path speed. In fact, 16 and 32 are not a good default anything. Is this making sense?

RobertBColton  

Ok, it wasn't just the color and alpha, it was also the scaling. So, anyway, the room background works in Game of Life now and the room tiles work in the Tile Collision Demo again. Those defaults are good enough for now. I'll probably do a run over the other protos later in another pull request so when the IDE creates new resources they have reasonable defaults.

Edit: Also, I was following you and agreed up until the end of your comment with the 16/32 shit. That's where you lost me and I don't know what you're talking about really.

RobertBColton  

Alright: 2d845ab

I found a typo you made which caused the player to spawn into the floor. With that fixed, we've come full circle and the Tile Collision Demo works great again.
Tile Collision Demo Fixed

RobertBColton  

Ok, so I caught another regression besides the fonts issue that is left. It occurs in the AI Tutorial GMX project I have which builds fine on master after two small unrelated changes. It seems to be related to the object access stuff you changed.

I am going to provide two downloads for this game because there are some issues with this project on master. We already provide angle_difference so I have to rename the script the user has by the same name. Also, there is a syntax error in the collision event of obj_unit_par because of the braces.

Download Original: AItutorial.gmx.zip
Download Patched: AITutorialPatched.zip

C:/Users/Owner/AppData/Local/ENIGMA/Preprocessor_Environment_Editable/IDE_EDIT_objectaccess.h: In function 'var& enigma::varaccess_i(int)':
C:/Users/Owner/AppData/Local/ENIGMA/Preprocessor_Environment_Editable/IDE_EDIT_objectaccess.h:117:7: error: duplicate case value
       case obj_unit_par: return ((OBJ_obj_unit_par*)inst)->i;
       ^~~~
C:/Users/Owner/AppData/Local/ENIGMA/Preprocessor_Environment_Editable/IDE_EDIT_objectaccess.h:116:7: note: previously used here
       case obj_unit_par: return ((OBJ_obj_unit_par*)inst)->i;
       ^~~~

AI Tutorial Object Access GML

RobertBColton  

Ok: 8e2f114

I've fixed another regression I caught earlier. I noticed the start screen of Wild Racing was red when the default window color is 0x000000FF (opaque black). This was just a mistake in the part where I map the settings and I needed to map the color, which was red because the endianess was reversed, using the javaColor helper.

RobertBColton  

So there is one more thing I have to discuss. This change is basically the ES2Proto changes I have been discussing. Since emake/RGM use the protocol buffers directly, and LGM uses EnigmaStruct which is now converted by GameData, there is no more need for Proto2ES and it can be deleted. So why haven't you deleted it yet?
JoshDreamland  

I wasn't going to take the liberty of deleting your code for you. If you'd like to include that deletion in this branch, go for it.
JoshDreamland  

Any lingering testing you have to do, please get it out of your system. The fundies is growing impatient. It must be fed.
RobertBColton  

Alright, I tested all of the games in my possession and only ran onto one additional regression. The game information has stopped working on this branch. We've been having discussions of deprecating it as a resource. I would like to add the "Notes" resources from GMS2, which is just plain text encoded. We've also talked about keeping game info but just letting you create unlimited ones for creating worklogs in the IDE. Maybe we could continue writing the first one in the tree for the engine for now though. Just letting you know, that's the only thing not working now in this pull request.
JoshDreamland  

My understanding is that you made the conscious decision not to support game information when you wrote the GameSettings copy logic. If you're having second thoughts, feel free to write it now. Or do it later. I think expecting us to ship a rich text viewer with a game engine is silly, regardless, so I won't miss it one way or another.
RobertBColton  

The test harness has two jobs that test loading projects that have timelines. One is a GMK and one is GMX.
RobertBColton  

@JoshDreamland alright, I had to make a specific test for timelines because I don't have any games, other than the CI tests we made too, that actually use them. These tests build in emake and LGM on master.

$ ./emake "C:/Users/Owner/Desktop/GMGames/timeline-test.gmk" -o /tmp/test.exe -r -e "Paths,Timelines" -w Win32
$ ./emake "C:/Users/Owner/Desktop/GMGames/timeline-test.gmx" -o /tmp/test.exe -r -e "Paths,Timelines" -w Win32

Download Timeline Test Projects: timeline-tests.zip

However, when checking out this branch, the projects don't build even under LGM:

C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.2.1/../../../../x86_64-w64-mingw32/bin/ld.exe: 
C:/Users/Owner/AppData/Local/ENIGMA/.eobjs/Windows/Windows/Mingw_GCC_G++/Run/SHELLmain.o:SHELLmain.cpp:
(.text$_ZN6enigma12event_parent27timeline_call_moment_scriptEii[_ZN6enigma12event_parent27timeline_call_moment_scriptEii]+0x10): undefined reference to `TLINE_time_0_MOMENT_25()'
C:/msys64/mingw64/bin/../lib/gcc/x86_64-w64-mingw32/8.2.1/../../../../x86_64-w64-mingw32/bin/ld.exe: 
C:/Users/Owner/AppData/Local/ENIGMA/.eobjs/Windows/Windows/Mingw_GCC_G++/Run/SHELLmain.o:SHELLmain.cpp:
(.text$_ZN6enigma12event_parent27timeline_call_moment_scriptEii[_ZN6enigma12event_parent27timeline_call_moment_scriptEii]+0x16): undefined reference to `TLINE_time_0_MOMENT_0()'

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