BelaPlatform / supercollider

an environment and programming language for real time audio synthesis and algorithmic composition
GNU General Public License v3.0
14 stars 8 forks source link

Bela-specific mods to the server class file #82

Closed giuliomoro closed 3 years ago

giuliomoro commented 3 years ago

Bela adds some options to SCClassLibrary/Common/Control/Server.sc (https://github.com/BelaPlatform/supercollider/commit/8932ff370f4e3a101c63a9d0c23a60c2f52660f1#diff-2f42f947dc1ce926c66c9ae9c0bf77cd7d1503f8a59121e2724d93f3b4db0863 and https://github.com/BelaPlatform/supercollider/commit/2fc8ce657613a24b5cecc5abe1776b5377af4bc6#diff-2f42f947dc1ce926c66c9ae9c0bf77cd7d1503f8a59121e2724d93f3b4db0863 ). If these get merged into the main supercollider branch, any server that is started from sclang will try to set those options and fail, unless it's running on Bela.

So with one merge we instantaneously break Supercollider for everyone else ... not sure it's the first impression we want to make :)

What's the best way to avoid this? If I understand the code correctly, we could avoid setting defaults for these parameters in the class file, this way they would not be added to the option list and it should work fine. Is there a better way of doing it?

@brianlheim @elgiano

mossheim commented 3 years ago

the easy solution IMO is to add a primitive which indicates sclang was compiled for bela support (if we can't already determine that programmatically) and only add those flags if it's true when constructing the server options list.

elgiano commented 3 years ago

How about subclassing Server on sclang side? We could then add and extend server options, their default values ~and override Server.default with a BelaServer.~ (probably a bad idea, how would we then tell if we need to override or not, i.e. if we are running sclang on a Bela or not? same problem as above :) )

mossheim commented 3 years ago

that could also be part of the solution. however keep in mind right now there is a hardcoded relationship between the server class in sclang and C++, that will have to be accounted for. in any case this shouldn't hold you back from making a PR.

btw. i recommend that we start that process within a week or two if you want this to be a part of 3.12.

giuliomoro commented 3 years ago

btw. i recommend that we start that process within a week or two if you want this to be a part of 3.12.

Thanks, will do, but I think we cannot show up with this server.sc that breaks everyone else's SuperCollider! @elgiano would you have time in the next few days to think of a solution for this? Again, I think an emergency fallback option could be to remove the default values, which in turn will not generate the custom options, but maybe there is something slightly nicer ... the best would be to retrieve the information from the server (though I am afraid it may require a lot of infrastructure?) and at this point we are probably looking for a quick win instead ...

mossheim commented 3 years ago

you already have two solutions described above. the first one is easiest and will completely fix the problem. is there something you find unsatisfactory about it?

elgiano commented 3 years ago

I'm actually looking into both, and instead of BelaServer I'm making BelaServerOptions. Then, in ServerOptions:-asOptionsString, if thisProcess.platform.hasFeature(\_Bela_isSupported), we add the options string from BelaServerOption. _Bela_isSupported would be a valid primitive only if sclang was compiled with the BELA flag on. The only thing that leaves me a bit unsatisfied is that we need to add Bela specific fields to ServerOptions anyway, or we have to change all code that says s.options.numAnalogInput causing a massive 100% breaking change.

I'll push a PR soon, please let me know if you see anything that could be done better in another way!

giuliomoro commented 3 years ago

@brianlheim is there something you find unsatisfactory about it?

Not really, it's just that I know nothing about sclang, so I did not understand much of the conversation, and I was asking @elgiano if they could attempt to make some practical steps in that direction, as time ticks!

_Bela_isSupported would be a valid primitive only if sclang was compiled with the BELA flag on.

If I understand correctly, this would put us in the situation where one could not download an official Sc release and use its IDE to live-code on Bela. That would be a step in the wrong direction I think: we want people to be able to use their Sc IDE with embedded server on their computer and use the same IDE to control Bela. Could it be that Bela_isSupported is set if sclang is compiled with the BELA flag on, OR set manually by the user?

elgiano commented 3 years ago

@giuliomoro That Bela primitive would be used to decide if you can pass Bela-options to a booting server. So the only case in which this has to be done, is when the server starts on Bela, and it can be started only from within Bela, as there's no way to boot a remote server. So, a user with a standard SC release would still be able to do everything with the Bela, except booting its server, which is already impossible anyway.

Hope I was clear enough :)

giuliomoro commented 3 years ago

Oh I see, that seems good, sorry if I was confused.

So this way would one still be able to tell sclang running on a host what the MaxScopeChannels option of a remote server is? (i.e.: would we be able to still run the belaScope remotely?)

PS: I like the idea of having the Bela options grouped either within s.options.bela.myOption or at least s.option.belaMyOption, to make it easier to distinguish what are the Bela-specific ones. I am not particularly concerned about breaking compatibility with earlier versions of Sc on Bela: it's a fairly easy fix for improved future-proofness.

elgiano commented 3 years ago

So this way would one still be able to tell sclang running on a host what the MaxScopeChannels option of a remote server is? (i.e.: would we be able to still run the belaScope remotely?)

It is still not possible to get remote servers' options. It will be possible to set it manually on the server object, but luckily that's not needed anymore with the new Scope implementation! :)

giuliomoro commented 3 years ago

nice!