Correct extension About.ey files #1012

Merged
merged 1 commit into from Sep 8, 2016

Conversation

Projects
None yet
2 participants
@RobertBColton
Member

RobertBColton commented Sep 8, 2016

There are two changes here. First, "ID" is being changed to "Identifier" for extensions as it is for the major systems so there is consistency. Second, "Name" was intended to be the human readable field as it is used in the major systems, but due to a bug that I just fixed in the plugin (enigma-dev/lgmplugin@ccc750a) "Identifier" was being used instead. So this PR swaps the role of "Identifier" and "Name".

Once this is merged, everybody is going to need the new ENIGMA plugin. Read the topic for more information about this issue.
http://enigma-dev.org/forums/index.php?topic=2718

Correct extension About.ey files
Name should be the human readable field and identifier should be what is
used in code. This patch requires the latest plugin which you can obtain
by reading my topic on the forum or through install.py.

@RobertBColton RobertBColton merged commit dd599c1 into enigma-dev:master Sep 8, 2016

@JoshDreamland

This comment has been minimized.

Show comment
Hide comment
@JoshDreamland

JoshDreamland Sep 28, 2017

Member

I can definitely get behind these changes, but are you certain nothing is using the ID field right now? Also, are we requiring that the "identifier" field is the same as the folder name? In that case, if nothing is using it, why not just get rid of it? It's just another weird artifact to maintain, that way.

My biggest concern is that this will break how LGM tells ENIGMA which extensions to use. As I said, if LGM is currently using the name of the folder to point ENIGMA to the correct extension, then we're fine, and I recommend deleting the identifier field altogether. My other concern is that ENIGMA might have been using it to generate C++ identifiers. In this case, the should have been an update to some compiler code to handle this change, or else, we seriously need to add some documentation about using the folder name for that.

The only argument I can see for keeping the identifier field is that, right now, there's nothing explicitly preventing spaces in the names of the extensions, while the "identifier" property seems to implicitly require no spaces.

We should figure out what codegen hinges on that assumption, if any, and verify that it is using the folder name (rather than Name), or update it to use this new field. Either way, we should document our decision. I believe there are Wiki pages about extensions and the ey files specifying them. If not, I'll write them. Just make sure we know how we're listing extensions, and how we're generating identifiers in code.

Member

JoshDreamland commented on 73a7170 Sep 28, 2017

I can definitely get behind these changes, but are you certain nothing is using the ID field right now? Also, are we requiring that the "identifier" field is the same as the folder name? In that case, if nothing is using it, why not just get rid of it? It's just another weird artifact to maintain, that way.

My biggest concern is that this will break how LGM tells ENIGMA which extensions to use. As I said, if LGM is currently using the name of the folder to point ENIGMA to the correct extension, then we're fine, and I recommend deleting the identifier field altogether. My other concern is that ENIGMA might have been using it to generate C++ identifiers. In this case, the should have been an update to some compiler code to handle this change, or else, we seriously need to add some documentation about using the folder name for that.

The only argument I can see for keeping the identifier field is that, right now, there's nothing explicitly preventing spaces in the names of the extensions, while the "identifier" property seems to implicitly require no spaces.

We should figure out what codegen hinges on that assumption, if any, and verify that it is using the folder name (rather than Name), or update it to use this new field. Either way, we should document our decision. I believe there are Wiki pages about extensions and the ey files specifying them. If not, I'll write them. Just make sure we know how we're listing extensions, and how we're generating identifiers in code.

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Sep 28, 2017

Member

Actually, both are currently being used. Name is used as the GUI label in LGM so that it's human readable, i.e. "Matrix Math" where spaces are allowed. Identifier is used by both the compiler and plugin to uniquely identify the extension with a valid C++ identifier, i.e. "MatrixMath" lacking a space. Before I changed anything, the engine used to use ID and the plugin used Identifier or vice versa, I forget, but the two were inconsistent.

My post was all about making the two consistent:
http://enigma-dev.org/forums/index.php?topic=2718

I can not verify to you at this time if anything else is still using/expecting it to be ID. I think I know where the confusion arose though, you put ID on your Wiki page about extensions:
http://enigma-dev.org/docs/wiki/index.php?title=Extensions&oldid=1715

The creator of LateralGM put Identifier in their Wiki page about extensions:
http://enigma-dev.org/docs/wiki/index.php?title=About.ey&oldid=5136

Which of the two of you was correct? I am guessing you, since ENIGMA is your thing. Also, leaving ID would have been a simpler change, so I probably should have just left it ID. Maybe you care to go back and undo my commits and just change the plugin to use ID? Right now, everything is consistently using Identifier though since my changes.

Member

RobertBColton replied Sep 28, 2017

Actually, both are currently being used. Name is used as the GUI label in LGM so that it's human readable, i.e. "Matrix Math" where spaces are allowed. Identifier is used by both the compiler and plugin to uniquely identify the extension with a valid C++ identifier, i.e. "MatrixMath" lacking a space. Before I changed anything, the engine used to use ID and the plugin used Identifier or vice versa, I forget, but the two were inconsistent.

My post was all about making the two consistent:
http://enigma-dev.org/forums/index.php?topic=2718

I can not verify to you at this time if anything else is still using/expecting it to be ID. I think I know where the confusion arose though, you put ID on your Wiki page about extensions:
http://enigma-dev.org/docs/wiki/index.php?title=Extensions&oldid=1715

The creator of LateralGM put Identifier in their Wiki page about extensions:
http://enigma-dev.org/docs/wiki/index.php?title=About.ey&oldid=5136

Which of the two of you was correct? I am guessing you, since ENIGMA is your thing. Also, leaving ID would have been a simpler change, so I probably should have just left it ID. Maybe you care to go back and undo my commits and just change the plugin to use ID? Right now, everything is consistently using Identifier though since my changes.

This comment has been minimized.

Show comment
Hide comment
@JoshDreamland

JoshDreamland Sep 28, 2017

Member

I'll side with Ism on this one—I prefer Identifier. Let's go ahead and use it (it's better to modify the compiler than the ENIGMA plugin, anyway).

Member

JoshDreamland replied Sep 28, 2017

I'll side with Ism on this one—I prefer Identifier. Let's go ahead and use it (it's better to modify the compiler than the ENIGMA plugin, anyway).

This comment has been minimized.

Show comment
Hide comment
@RobertBColton

RobertBColton Sep 28, 2017

Member

Sounds good, we are consistent now then, and just to be clear, the Wiki article you started has since been updated to better reflect this consistently:
http://enigma-dev.org/docs/Wiki/Extensions
http://enigma-dev.org/docs/wiki/index.php?title=Extensions&action=historysubmit&diff=30835&oldid=30702

Member

RobertBColton replied Sep 28, 2017

Sounds good, we are consistent now then, and just to be clear, the Wiki article you started has since been updated to better reflect this consistently:
http://enigma-dev.org/docs/Wiki/Extensions
http://enigma-dev.org/docs/wiki/index.php?title=Extensions&action=historysubmit&diff=30835&oldid=30702

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment