Fix build props and improve docs #419
Conversation
- Binaries - BinariesNET8
- English - Simpler
|
And I don't know if someone is still using this build props? As I said in CnCNet/xna-cncnet-client#516 (comment), the filename is wrong so the file cannot be picked by the client's props. |
| @@ -8,7 +8,7 @@ so that the client starts up with the YR theme. | |||
| <PropertyGroup> | |||
| <!-- | |||
| Change the value of YRSource to wherever you have the YR client repo checked out OR | |||
| you can use your installation path of YR for cncnet. | |||
There was a problem hiding this comment.
this is fine as is, i think
There was a problem hiding this comment.
hey, @brichardson1991! thank you for reviewing my changes.
Change the value of YRSource to wherever you have the YR client repo checked out OR you can use your installation path of YR for cncnet.
the original sentence sounds a bit odd to my non-native ears. that sentence, i think, means that we have two choices about specifying the path:
- the one is my checked-out YR client repo.
- the other is my YR installation.
to be simplified, that's "change A to B or C" or "change A to B or change A to C". here the "you can use" looks being placed in a wrong location, so i've deleted it.
your opinion?
There was a problem hiding this comment.
Pretty much yes, we are stating use the one where you've just checked the repository out to or your game installation directory.
The language used at present is fine, but I guess for non-english native it's a bit harder to interpret.
The easiest thing would be
<!--
Change the value of YRSource to wherever you have the YR client repo checked out OR
you can use your installation path of YR for cncnet. For example:
C:\repos\cncnet-yr-client-package\ or
C:\SteamLibrary\steamapps\common\Command & Conquer Red Alert II\
--!>
There was a problem hiding this comment.
Thanks. Adding examples looks better.
Maybe I can rewrite it like this (comparing with the original):
<PropertyGroup>
<!--
Change the value of YRSource to wherever you have the YR client repo checked out OR
you can use your installation path of YR for cncnet.
- This path should be the same one that contains the file "gamemd.exe".
- Example: c:\repos\cncnet-yr-client-package\package
+ Examples:
+ - c:\repos\cncnet-yr-client-package\package
+ - c:\Westwood\RA2
-->
<YRSource></YRSource>
<!--
OPTIONAL
Change the value of YRGameInstallSource to wherever you have the game installed locally.
- This is only necessary if YRSource above is not already set to the game installation path and
+ This is only necessary if YRSource above is not already set to the game installation path (containing "gamemd.exe") and
you need to be able to launch the game from the client during development/while using the YR client repo for YRSource.
Example: c:\ra2InstallPath\Command and Conquer Red Alert II
-->
<YRGameInstallSource></YRGameInstallSource>
Explanation:
- "c:\Westwood\RA2": I think that the game from Steam is not a "YR for cncnet" without modification, so I use the default installation path of "CnCNet5_YR_Installer.exe". The code is at
- About "gamemd.exe": we have no game files in YR client repo, so don't mislead new developers to try to copy them to the "package" directory in this repository and then face a bunch of untracked files. Yeah, I did it before.
- Lowercase drive letters, missing trailing backslashes and other changes: consistent style.
Do you like this? 😁
Incorrect copy destination?Another question: why do we need a <Target Name="CopyYRResources" AfterTargets="PrepareForRun" Condition="$(ProjectName) == 'DXMainClient'">
<PropertyGroup>
...
- <YRStartupPath>$(OutputPath)..\..\..</YRStartupPath>
</PropertyGroup>
In Common MSBuild Project Properties - MSBuild | Microsoft Learn:
I've tested the "YRWindowsDXDebug" configuration and the directory structure is as below: Here I think we should change |
|
@GrantBartlett @alexp8 not sure about the last comment made by @pzhlkj6612, can you elaborate for him |
|
Reason: several years ago, when the client was still in .NET 7, the client was not able to automatically find the Resource folder. Instead, it assumed a path like Resources/Binaries/Windows/clientdx.dll. I think either removing it or retaining it with a proper XML comment should be fine. |
Hmm, no. The current code cannot find resources in
That's why I said the above. And please also see CnCNet/xna-cncnet-client#632, I think we may have things didn't get migrated during refactor or upgrade. |
Hi! I've been trying building YR CnCNet by myself these days and have found something can be improved. Please see the commits for details.