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

Unity(JUMBO) build support #307

Merged
merged 21 commits into from
Nov 24, 2023
Merged

Unity(JUMBO) build support #307

merged 21 commits into from
Nov 24, 2023

Conversation

Say-Y
Copy link
Contributor

@Say-Y Say-Y commented Nov 1, 2023

I'm familiar with Jumbo build, so I think it would be nice to have this feature integrated.

#305
Apply modifications

@jspelletier
Copy link
Collaborator

Could modify one of the existing sample to add jumbo build support in one of the configuration at least?

Copy link
Contributor Author

@Say-Y Say-Y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unity -> Jumbo

- Add JumboBuild Sample
Copy link
Contributor Author

@Say-Y Say-Y left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Add JumboBuild Sample based on HelloWorld Sample
  • Add JumboBuild options

Please let me know if there is anything that needs to be corrected.

@sylvainfortin
Copy link
Collaborator

Hi @Say-Y. Since you are adding a new sample, could you add it to SamplesDef.json. Once added to this file, it will be automatically be added to CI along other samples. See https://github.com/ubisoft/Sharpmake/blob/main/docs/Samples.md.

@Say-Y
Copy link
Contributor Author

Say-Y commented Nov 6, 2023

Hi @jspelletier , I added a few header and cpp files. (test1.h, test1.cpp, test2.h, test2.cpp)

Hi @sylvainfortin , 'JumboBuild' was added by referring to the 'HelloWorld' part of the 'SamplesDef.json' file. also removed JumboBuild's 'reference' folder.
And I don't need to worry about 'UpdateSamplesOutput.bat'?

@jspelletier
Copy link
Collaborator

Hi @jspelletier , I added a few header and cpp files. (test1.h, test1.cpp, test2.h, test2.cpp)

Hi @sylvainfortin , 'JumboBuild' was added by referring to the 'HelloWorld' part of the 'SamplesDef.json' file. also removed JumboBuild's 'reference' folder. And I don't need to worry about 'UpdateSamplesOutput.bat'?

UpdateSamplesOutput.bat is used to generate the reference files

@Say-Y
Copy link
Contributor Author

Say-Y commented Nov 6, 2023

Hi @jspelletier , I added a few header and cpp files. (test1.h, test1.cpp, test2.h, test2.cpp)
Hi @sylvainfortin , 'JumboBuild' was added by referring to the 'HelloWorld' part of the 'SamplesDef.json' file. also removed JumboBuild's 'reference' folder. And I don't need to worry about 'UpdateSamplesOutput.bat'?

UpdateSamplesOutput.bat is used to generate the reference files

Should I also edit that file?

if not "%ERRORLEVEL_BACKUP%" == "0" goto error
call :UpdateRef samples JumboBuild                  JumboBuild.sharpmake.cs                    reference         JumboBuild

SamplesDef.json Outdated Show resolved Hide resolved
@Say-Y
Copy link
Contributor Author

Say-Y commented Nov 10, 2023

I have applied your request.
thank you

@jspelletier
Copy link
Collaborator

jspelletier commented Nov 17, 2023

I ran the CI.
CI failed because you need to run the .bat to create the reference files. They need to be added to your PR.

@Say-Y
Copy link
Contributor Author

Say-Y commented Nov 17, 2023

I have confirmed that UpdateSamplesOutput.bat fails without the following settings

AddTargets(new Target(
        Platform.win32 | Platform.win64,
        DevEnv.vs2019,
        Optimization.Debug | Optimization.Release
));

After the change, I confirmed that regression_test.py passed.

@jspelletier jspelletier merged commit 5958ff6 into ubisoft:main Nov 24, 2023
48 checks passed
@Say-Y Say-Y mentioned this pull request Nov 25, 2023
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.

3 participants