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

Maya + Unreal Build scripts (Win/Linux) #15

Merged
merged 43 commits into from
May 28, 2024

Conversation

Lypsolon
Copy link
Contributor

@Lypsolon Lypsolon commented May 8, 2024

im opening this pr to close the other Pr
as this branch was created from the other branch and contains all the important work from it.
this PR also serves to block further development from me until both plugins are fully working and tested for both Os setups.

this PR implements

  • Unreal5_4 win + Linux (Ps: win is not well tested yet so assume fixes there)
  • Maya24_4 win + linux (Ps: win is not well tested yet so assume fixes there)

@Lypsolon Lypsolon requested a review from antirotor May 8, 2024 15:45
@Lypsolon Lypsolon mentioned this pull request May 8, 2024
@m-u-r-p-h-y
Copy link
Member

Is Ondra the only one to help here or can we test something from a user perspective as well? If so, we need some instructions :)

@Lypsolon
Copy link
Contributor Author

Lypsolon commented May 8, 2024

Is Ondra the only one to help here or can we test something from a user perspective as well? If so, we need some instructions :)

im not sure tbh right now.
the main thing on this branch is resolver loading on the Usd side and the compile time linking errors that might be there on the windows side.

currently there is no User perspective on this repo because the user interaction / server settings will be handled by the AyonUsd plugin.

so im gonna answer that with an: im not sure.

@Lypsolon Lypsolon requested review from antirotor and removed request for antirotor May 20, 2024 06:43
Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Running test/TestAyonUsd.bat:

(base) PS C:\Users\annat\Documents\Projects\Ayon\ayon-usd-resolver> .\scripts\buildAyonUsd.bat

C:\Users\annat\Documents\Projects\Ayon\ayon-usd-resolver>REM in case you use Pyenv // Pyenv exec works

C:\Users\annat\Documents\Projects\Ayon\ayon-usd-resolver>cd ../

C:\Users\annat\Documents\Projects\Ayon>rmdir /s /q build
The system cannot find the file specified.

C:\Users\annat\Documents\Projects\Ayon>rmdir /s /q Resolvers/AyonUsdWin/AyonUsd23_5_py39
Invalid switch - "AyonUsdWin".

C:\Users\annat\Documents\Projects\Ayon>set AyonUsdRoot=

C:\Users\annat\Documents\Projects\Ayon>set COMPILEPLUGIN=

C:\Users\annat\Documents\Projects\Ayon>cmake -S . -B build -DDEV=0 -DJTRACE=0 -DCMAKE_BUILD_TYPE=Release
CMake Error: The source directory "C:/Users/annat/Documents/Projects/Ayon" does not appear to contain CMakeLists.txt.
Specify --help for usage, or press the help button on the CMake GUI.

C:\Users\annat\Documents\Projects\Ayon>cmake --build build --clean-first --config Release
Error: C:/Users/annat/Documents/Projects/Ayon/build is not a directory

C:\Users\annat\Documents\Projects\Ayon>cmake --install build
CMake Error: Error processing file: C:/Users/annat/Documents/Projects/Ayon/build/cmake_install.cmake

Sure it was as naive as running it directly from within developer console, but maybe we should add some sanity checks. But in next PR - just mentioning it here.

Also, the script are expecting that you will run them from specific directory - like /scripts/build.bat has to be run from within /scripts otherwise it will fail.

src/AyonUsdResolver/CMakeLists.txt Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
BuildPlugins/AyonUsdWin/AyonUsd23_5_py39.cmake Outdated Show resolved Hide resolved
BuildPlugins/MayaLinux/LinuxPy310Maya2024_2.cmake Outdated Show resolved Hide resolved
BuildPlugins/MayaLinux/LinuxPy311Maya2025.cmake Outdated Show resolved Hide resolved
BuildPlugins/MayaWin/WinPy310Maya2024_2.cmake Outdated Show resolved Hide resolved
@antirotor antirotor added the type: enhancement Enhancements to existing functionality label May 24, 2024
Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

see comments above

Lypsolon and others added 17 commits May 27, 2024 12:28
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
@Lypsolon
Copy link
Contributor Author

Lypsolon commented May 28, 2024

COMPILEPLUGIN
@antirotor

1:
I believe the current separation is better than unifying them into one folder.
My reason for that is that having all the resolvers inside a folder can make it hard to find things if you build, for eg
- win, Alma9, Alma8, CentOs7, CentOsStream9, Ubunto, etc
I also want to note: that we currently only support Houdini 19.5 and 20, and with the current separation, this already creates 4 files in the HouLinux folder; if we add 2 more versions, e.g. 20.5, and 21, we will end up with 8 files. if we now unify Windows and Linux foulders then we have 16 files, 8 of those files do not matter to you if you're only working on one platform at a time, and that's mostly the case; even I don't care for the Windows files when on Linux and the other way around.
So, in the end, I don't see a disadvantage in having them separated the way they are right now, but I can see an annoyance in having them all bundled up. Do you have a reason for wanting that change?

2:
I'm not sure if I'm following TBH. As I see it, the naming is unified ( {platform}{PythonVersoin}{SoftwareVersion} ). I personally avoid . and - in naming schemes as much as I can. We could add underscores between the parts of the name, but I don't know if that would be a big advantage. if you have a naming scheme you would like let me know what it is and why you would like it.

@Lypsolon Lypsolon requested a review from antirotor May 28, 2024 12:08
@antirotor
Copy link
Member

Do you have a reason for wanting that change?

Nothing really important, just to avoid duplication of information in dir/file hierarchy. So you basically merge {platform} in {app}{Platform}/{Platform}{Python}{App}{app_version}.cmake`. But you have point in number of files in one directory and anyway, it is not that big issue, more like personal preference.

I'm not sure if I'm following TBH. As I see it, the naming is unified ( {platform}{PythonVersoin}{SoftwareVersion} )

By unification, I meant:

HouWin/WindowsPy310Houdini20.cmake
vs.
MayaWin/WinPy311Maya2025.cmake

(notice Windows vs. Win)

I guess it is problem just for *HouWin scripts and I vote for shorter win. I don't mind came case or snake case - again, it is more of personal preference.

I think dashes and underscores actually help with readability and they are used everywhere - in package repositories, like:

LibRaw-0.20.2-6.el9.x86_64.rpm (centos9)
wayland-1.22.0.ebuild (gentoo)
libraw_0.20.2-1+deb11u1.dsc (debian)

Copy link
Member

@antirotor antirotor left a comment

Choose a reason for hiding this comment

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

Just those little changes

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Lypsolon and others added 2 commits May 28, 2024 16:09
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
Co-authored-by: Ondřej Samohel <33513211+antirotor@users.noreply.github.com>
@Lypsolon
Copy link
Contributor Author

Do you have a reason for wanting that change?

Nothing really important, just to avoid duplication of information in dir/file hierarchy. So you basically merge {platform} in {app}{Platform}/{Platform}{Python}{App}{app_version}.cmake`. But you have point in number of files in one directory and anyway, it is not that big issue, more like personal preference.

I'm not sure if I'm following TBH. As I see it, the naming is unified ( {platform}{PythonVersoin}{SoftwareVersion} )

By unification, I meant:

HouWin/WindowsPy310Houdini20.cmake vs. MayaWin/WinPy311Maya2025.cmake

(notice Windows vs. Win)

I guess it is problem just for *HouWin scripts and I vote for shorter win. I don't mind came case or snake case - again, it is more of personal preference.

I think dashes and underscores actually help with readability and they are used everywhere - in package repositories, like:

LibRaw-0.20.2-6.el9.x86_64.rpm (centos9) wayland-1.22.0.ebuild (gentoo) libraw_0.20.2-1+deb11u1.dsc (debian)

Okay, perfect, I see what you mean.
I would propose to have an extra PR for fixing the naming setup just to have it nice and clean and maybe because I'm afraid off merge confilkts.
in that case, I would say renaming to
{AppName}{platform}/{platform}{AppName}{AppVersion}Py{PythonVersion}.cmake
and for platform I would say (win, linux, mac) as base options but that might need to be extended with things like centOs7 or so if we need specific builds that are linux but need special care.
I would still like to keep them in the folders like they are right now because, like I said, I find it annoying to see a lot of files that I don't need at the moment.

@antirotor antirotor merged commit b298ca6 into develop May 28, 2024
@antirotor antirotor deleted the bug/AY-5245_UnrealBuildNotWorking branch May 28, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants