search

asb2m10 / dexed

DX7 FM multi plaform/multi format plugin
GNU General Public License v3.0
2.86k stars 245 forks source link

Corrections of unhandled exceptions/illegal file contents and sizes related to some ''.scl'' and ''.kbm'' files #417

Closed FulopNandor closed 7 months ago

FulopNandor commented 7 months ago

Dear Pascal, do apply my pull request, please. Its purpose is to handle errors that occur when certain ''.scl'' and/or ''.kbm'' files are opened, causing Dexed to crash. Furthermore, it contains some minor modifications related to displaying the content of ''.scl''/''.kbm'' files in the ''Current Tuning'' dialog, and exported ''.csv'' files. To achieve this, not only some source files of Dexed were modified, but the ''tuning-library'' repository has also been updated to the latest version. Additionally, a temporary repository from ''surgesynthteam_tuningui'' has been introduced to contain the necessary modifications. A more detailed description of the encountered errors and a their corrections can be found in the announcement . Thank you! (I apologize for the mistake, I meant to write ''Pascal'' instead of ''Paul''. Thank you for your understanding.)

asb2m10 commented 7 months ago

Thanks for this PR. Any chance you get surgesynthteam_tuningui PR pushed on surge-synthesizer ?

FulopNandor commented 7 months ago

Thanks for this PR. Any chance you get surgesynthteam_tuningui PR pushed on surge-synthesizer ?

You are welcome, I am glad if you have found it useful. Yes, I wish to send a PR to @baconpaul at Surge Synthesizer Team, to accept my modifications. And, if they accepted, I wish to restore the references to the related submodul to surge-synthesizer/surgesynthteam_tuningui repository, as they are the original authors. I really do not wish to alter the original structure of the Dexed repository. What is your recommendation, shall I send a PR to Surge Synthesizer Team now, or shall I wait for a while for feedback from other developers?”

baconpaul commented 7 months ago

I don’t see the pr. Can you link it here?

baconpaul commented 7 months ago

And would be happy to upstream those changes. They look pretty good on a quick skim.

on the dexed changes hasn’t juce::ScopedPtr gone out of style and now we use Std::unique instead?

FulopNandor commented 7 months ago

I don’t see the pr. Can you link it here?

I have not initiated the PR yet, I am waiting for Pascal's answer, maybe he has some issues to address. But of course, I can provide a link to my temporary repoI: https://github.com/FulopNandor/surgesynthteam_tuningui. I am intending to send a PR to you at its present state - except if Pascal has some further advice.:-)

FulopNandor commented 7 months ago

And would be happy to upstream those changes. They look pretty good on a quick skim.

on the dexed changes hasn’t juce::ScopedPtr gone out of style and now we use Std::unique instead?

Thank you for your notification! I am going to try out!

baconpaul commented 7 months ago

Cool. Except for the readme change your changes to timing ui look spot on to me in a quick review. And I would happily merge them

I can definitely imagine pascal preferring that too since the surge synthesizer git dep has multiple maintainers team structure etc

and i would welcome the contribution!

asb2m10 commented 7 months ago

I don’t see the pr. Can you link it here?

I have not initiated the PR yet, I am waiting for Pascal's answer, maybe he has some issues to address. But of course, I can provide a link to my temporary repoI: https://github.com/FulopNandor/surgesynthteam_tuningui. I am intending to send a PR to you at its present state - except if Pascal has some further advice.:-)

That is fine for now and thanks for adapting the unique_ptr. Once surgesynthteam_tuningui is merged on the surge repo, I will merge this PR. Thanks.

FulopNandor commented 7 months ago

Dear Pascal (@asb2m10) and Paul (@baconpaul), thank you for your help and patience! I have just sent a PR to Paul.

baconpaul commented 7 months ago

Cool. So glad this code is still getting used in dexed! Thanks for the contribution @FulopNandor

FulopNandor commented 7 months ago

Dear Pascal (@asb2m10) and Paul (@baconpaul), I am glad that you found my modifications useful and incorporated them into your works. Wow, I have just made it through my first repos, commits and PRs on GitHub... ...thank you very much for your kind support!:-)