search

openAVproductions / openAV-Luppp

Luppp is a live performance tool, created by OpenAV productions.
http://openavproductions.com/luppp
GNU General Public License v3.0
259 stars 45 forks source link

Replace all hard coded scene numbers #213

Closed georgkrause closed 6 years ago

georgkrause commented 6 years ago

In this PR i replaced all hard coded Scene numbers by NSCENES from the config.h. This way the number of tracks and scenes can be changed on compile time.

This is the first step in getting luppp running with custom number of tracks and scenes. Currently, it is working, but the GUI gets wrong. But this should be another, following PR. I hope i found all hard coded numbers, but luppp seems to work with custom values so i guess i found everything. I also searched for 8 and 10 in the hole repository and replaced everything.

There is just one file left: /home/georg/workspace/openAV-Luppp/src/tests/lupppTestMaterial/lupppTest/session.luppp which contains hard coded number of sceneNames and tracks. But there is no easy way to repair this. (In this moment it gets in my mind to try to load this session with changed number of tracks and scenes to see what happens.)

georgkrause commented 6 years ago

luppp10-4 Luppp with 10 tracks and 4 scenes.

georgkrause commented 6 years ago

Luppp crashes if one loads a session which is stored with more tracks OR scenes. This needs also to be fixed. Here or in another PR?

harryhaaren commented 6 years ago

No PR should ever introduce the opportunity to crash / fail. So yes, this should be handled here, in this PR.

harryhaaren commented 6 years ago

Hold on a sec - you're not actually changing it? You're just improving the code to actually use the #define values. That's fine as a PR itself. Apologies, I should have reviewed the code before commenting.

georgkrause commented 6 years ago

This PR does not really introduce this opportunity, it just makes it possible to run luppp with others than 8 tracks or 10 scenes. To do this, you would need to change the config.h and build again. If you do this without this PR, luppp crashes. If you do this after this PR, it only crashes if you load an old session.

So this PR shouldnt be an problem for the normal user.

harryhaaren commented 6 years ago

Right - so is it ready to merge?

georgkrause commented 6 years ago

Oh, didnt saw your comment. nvm ;)

Since you like the politic of small steps and i feel really comfortable with going them, i would like to merge this and take the next step by focus on the shown GUI-Issues (we cannot show this to any user)

harryhaaren commented 6 years ago

Ah yes - marking with the "please merge" label is a good idea, I won't merge things from you without that label - that way I know when you're ready :)

georgkrause commented 6 years ago

Yep, came to my mind when we had the merge of the not ready PR last time, we should use all Github-Features we have ;) Thanks for merging :)