nurpax / petmate

PETSCII editor with Electron/React/Redux
MIT License
181 stars 14 forks source link

add ability to give screen tabs a name #89

Closed nurpax closed 5 years ago

nurpax commented 6 years ago

This would also be exported in .asm export.

Would make it easier to use exported PETSCII in an assembly project.

nurpax commented 5 years ago

Here's how it looks like in HEAD: https://youtu.be/x4_ShSdPghI

nurpax commented 5 years ago

Summoning @Esshahn & @Viza74 for some design feedback.

So the basic functionality for setting a name for a screen was just added. I didn't add actually using these names in .asm export yet.

I intended the names to be mainly used for assembly export. E.g., the user can set a name for a tab and then that name becomes its symbol name in the .asm output. Should make it a bit easier to work on content that uses screens exported by Petmate.

OK, so the questions:

I'm thinking the .asm export will by default use these names for symbols but maybe there can be an export dialog option to just use frame0000, frame0001.

Esshahn commented 5 years ago

Should the names then be restricted to [_a-zA-Z]+[_a-zA-Z0-9]* so that the .asm file compiles? Or should there be some mangling to turn any string into a compilable output (eg., convert spaces to under scores, etc.)? If the former, then the input form should validate the entered string while typing and show some tooltip if it's not valid.

I would not put effort into it, especially since some assemblers have different syntax restrictions. Let the users name it whatever they like.

I now show unset names as "Untitled". But perhaps I should always give them a name? Say "frame0000" (like in Marq's PETSCII). Or alternatively, if the name is not set, don't show it at all. This would be make the UI look slightly less busy but at the cost of reduced UI discoverability (you won't know to click below thumbnails if the text is not there.)

I'd recommend using e.g. "screen_001" and increment from there. If you want to make it even more useful, introduce a setting in the preferences, like

Default Name: ____

Viza74 commented 5 years ago

I don't have strong opinions on this :) Go for the easiest to implement route, and work up from there :) I would say asm/c restricted names in the entry field (becasue I guess validation is a part of react... or at least it should be, I don't know react :) ) For the default names, I would say difinitely something unique for every new screen, but I wouldn't add an option to choose the base name, just pick "frame" or "screen" and thats it.

nurpax commented 5 years ago

Yes, let's not have an option for setting a "default screen name".

As for validation: artist folks might want to use non-assembler names for their screens. This feature is not strictly for just asm coders. :) I think what I will do is to auto-convert spaces to underscores or something like that. Maybe add a code comment in the generated .asm saying where the names came from and how to change them.

Viza74 commented 5 years ago

I image that the naming feature would be used for exporting purposes like 90% of the time, so my care for the artist is around 10% :) As an artist what is the purpose of the screen naming? If one do animation, mostly does'nt care about frame names. If used for multiple artworks in the same file - there would'nt be so much in one file that naming is critical. Allowing non-source-code friendly characters would be just a source of confusion for the workflow if used in any program. But thats just my viewpoint, could be dead wrong. Maybe petmate is tool for artist and most of the things made with it never make it to an actual program (or if it is, that is done with the built in export to some runnable format)

nurpax commented 5 years ago

@Viza74 so react doesn't have built-in form validation out-of-the-box (although it's not too hard to do.) But looks like HTML was powerful enough to express that.. Easiest change ever :)

https://github.com/nurpax/petmate/commit/2a24e130323c2cdc62180a9a10c062df1812c0d3