Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Importing and exporting with custom text formats #82

Merged
merged 13 commits into from
Sep 1, 2019

Conversation

jpcima
Copy link
Collaborator

@jpcima jpcima commented Aug 15, 2019

#81

Hi, this shows a concept of using custom text formats for reading and writing.
It's not implemented yet in the editor, and only has VOPM as a format now.

In createVopmFormat, it can be seen how to build a grammar for a format.
The Vals are to replace with the actual values of the instrument. To associate with values, it relies on MetaParameter, which has been given a setter function.
The Ints are just number fields which are ignored, and have defaults used for writing.

While writing, the whitespace tokens are used for human-readable formatting.
During parsing, the whitespace tokens are always ignored.

@jpcima
Copy link
Collaborator Author

jpcima commented Aug 31, 2019

Abount integrating this in the editor, I an not completely sure how I lay it out the best way.
I think a small toolbar which has 3 icons: copy/paste/select format.
Maybe I can take some space for it at the right side of the long instrument name box.

Another thing is that there exists already ctrl-c/ctrl-v as copy & paste, which currently copies instrument's binary data.
It's important to keep these two are distinct, as custom format are unable to save all data. But also this difference can be lead to confusion for users.

@jpcima jpcima marked this pull request as ready for review August 31, 2019 21:38
@jpcima
Copy link
Collaborator Author

jpcima commented Aug 31, 2019

What is implemented and working:

  • VOPM/FMP/PMD instrument formats, allowing reading and writing with the text format
  • a menu in the user interface to permit copy/paste actions, and format choice
  • remembering the selected format in user settings

@jpcima
Copy link
Collaborator Author

jpcima commented Sep 1, 2019

Next, I allowed to parse varying forms of PMD found...
Ther are some using comma to separate, and some which don't.

Also, writing name seems optional is the case of PMD.
I have extended the parser with conditionals using simple looking ahead by one token.
It looks for an = symbol to check the presence of name.

For me, the current state if fine to merge.

@jpcima
Copy link
Collaborator Author

jpcima commented Sep 1, 2019

Also @Wohlstand, the travis shows failure at the ftp upload step.

@Wohlstand
Copy link
Owner

That because your branch state lacks the script I have added in some moment, so, merging...

@Wohlstand Wohlstand merged commit 0b38f2c into Wohlstand:master Sep 1, 2019
@jpcima
Copy link
Collaborator Author

jpcima commented Sep 1, 2019

Oh that's right, I didn't pay attention about this. Thanks

atsushieno added a commit to atsushieno/OPN2BankEditor that referenced this pull request Jul 25, 2020
context: Wohlstand#81 / Wohlstand#82

In FMP distribution from 199x, there was a default template bank
definition file (`88NEIRO.TPI`) whose format was like this:

```
"Harpsichord
'@ 0
   AR DR SR RR SL TL KS ML DT
'@ 31,12, 4,10, 1,32, 0,12, 0
'@ 31, 2, 4, 6,15,57, 3,15, 1
'@ 31,12, 4, 6, 0,30, 0, 1, 0
'@ 31, 5, 7, 7, 2, 0, 2, 3, 4
'@ 2,7
```

On the other hand, this is what we see from "Copy to FMP":

```
'@ FA 0
'@   0,  0,  0,  0,  0,  0,  0,  0,  0,   0
'@   0,  0,  0,  0,  0,  0,  0,  0,  0,   0
'@   0,  0,  0,  0,  0,  0,  0,  0,  0,   0
'@   0,  0,  0,  0,  0,  0,  0,  0,  0,   0
'@   0,  0
```

There are some notable differences, but so far what I find annoying is
that the last parameter (AM/DT2) was optional in the original format,
while it was mandatory without this fix.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants