suggest pro when trying to open source with standard #1684
Conversation
CMakeLists.txt
Outdated
| @@ -748,9 +748,9 @@ set(TIC80STUDIO_SRC | |||
| ${TIC80LIB_DIR}/ext/png.c | |||
| ) | |||
|
|
|||
| if(${BUILD_PRO}) | |||
| #if(${BUILD_PRO}) | |||
| set(TIC80STUDIO_SRC ${TIC80STUDIO_SRC} ${TIC80LIB_DIR}/studio/project.c) | |||
There was a problem hiding this comment.
This is needed because we need tic_project_ext unless you wanted to factor that out into tools... I'm not sure what the harm is in building this file though - only the single function should actually get linked during compile.
CMakeLists.txt
Outdated
| @@ -738,6 +738,7 @@ set(TIC80STUDIO_SRC | |||
| ${TIC80LIB_DIR}/studio/editors/sfx.c | |||
| ${TIC80LIB_DIR}/studio/editors/music.c | |||
| ${TIC80LIB_DIR}/studio/studio.c | |||
| ${TIC80LIB_DIR}/studio/project.c | |||
There was a problem hiding this comment.
project.c should only be built in Pro version, why did you add it here?
There was a problem hiding this comment.
I saw below you added this for the sake of tic_project_ext() function, maybe better move it to the src/tools.c
There was a problem hiding this comment.
I could... but curious, what is the harm in building it if it's never linked (other than tic_project_ext)? I don't see any real harm. What makes it PRO is the define being turned on and enabled, not just whether the project.c file is compiled or not...
There was a problem hiding this comment.
Why should we include it in the project if its code is not used in the PRO build?
Another way, you can add it but pls wrap all the functions with
#if defined(TIC80_PRO)
...
#endifThere was a problem hiding this comment.
if its code is not used in the PRO build?
But it's code IS used in the build now... so including it makes sense. The functions that aren't referenced are never included always because the linker will just silently discard them.
You're way overthinking this one, but I went ahead and just moved the function to tools...
This reverts commit 366aaac.
|
|
|
Looks good, thank you. |
Closes #1643.