make default x64 and fix .dd imported files #2693
Conversation
| <AllowedOutputExtensionsInPackageBuildOutputFolder>.so; .a; $(AllowedOutputExtensionsInPackageBuildOutputFolder)</AllowedOutputExtensionsInPackageBuildOutputFolder> | ||
|
|
||
| <StrideNativeWindowsArm64Enabled Condition="'$(StrideNativeWindowsArm64Enabled)' == ''">true</StrideNativeWindowsArm64Enabled> | ||
| <StrideNativeWindowsArm64Enabled Condition="'$(StrideNativeWindowsArm64Enabled)' == ''">false</StrideNativeWindowsArm64Enabled> |
There was a problem hiding this comment.
Isn't that change forces to set to True when releasing nuget packages?
There was a problem hiding this comment.
What do you mean? Do you mean, does this get affected by the team city builds?
There was a problem hiding this comment.
I .. think so, since by default is true then it's generating arm64 dll files without any additional paramateres. I'm just afraid that's gonna require some additional modification.
There was a problem hiding this comment.
My proposal would be something like this
| <StrideNativeWindowsArm64Enabled Condition="'$(StrideNativeWindowsArm64Enabled)' == ''">false</StrideNativeWindowsArm64Enabled> | |
| <StrideNativeWindowsArm64Enabled Condition="'$(StrideNativeWindowsArm64Enabled)' == ''">true</StrideNativeWindowsArm64Enabled> | |
| <StrideNativeWindowsArm64Enabled Condition="'$(StrideNativeWindowsArm64Enabled)' == '' AND '$(Configuration)' != 'Release'">false</StrideNativeWindowsArm64Enabled> |
There was a problem hiding this comment.
Would be great if someone else comment If I'm correct
There was a problem hiding this comment.
If it's easy to configure in TC then why not ,otherwise I'd go with my change (i.e if value is NaN then set true, then if is not relase, dont geneerate arm files)
There was a problem hiding this comment.
It could be irritating that the debug build works and once you switch to release it breaks. I'd prefer passing the parameter explicitly to TC build.
There was a problem hiding this comment.
Im not very familiar with conditionals in targets files. If we make the change through TC then would my current change be ok currently or would it cause a problem?
There was a problem hiding this comment.
It should be ok. I think the file we should modify is build/Stride.build which contains a property BuildProperties, adding StrideNativeWindowsArm64Enabled=true should enable it for command line / TC builds.
|
The changes in DXTWrapper were made on purpose. Without them the calls would not work on arm64. Are you saying they don't work now on x64? |
|
Yes, after applying the changes, it seems that changed methods stopped working (skybox asset for example) on win |
|
Interesting. Then I'll have to read up on it again and find a different solution. |
|
I might be able to use an if def in the class with a const to change the value depending on architecture. |
|
@azeno would you be able to test out this branch when you have some time to make sure it still works for windows arm? I just added a check for the value you added in the targets file |
|
@Doprez Sure can do, but I was just wondering if we should change the native code so that it uses Unicode like other methods in that file do. Those ifdefs are such a pain. |
|
100% hate what I did here lol I am not a fan of them either. I just wasn't sure on the fix complexity with the native code so this was a quick work around fix. |
|
Let me check the native part .. |
|
@Doprez Looks like simply re-compiling |
|
Found an alternative to detect CPU architecture instead that I think should work better assuming I did it right. Will make a new PR that should be simpler. |
PR Details
Changes the default build setting changed by #2644
Also removes a change in the DXTWrapper
Related Issue
None, found in master.
Types of changes
Checklist