CnCNet / xna-cncnet-client

XNA / MonoGame based client for playing classic Command & Conquer games both online and offline with a CnCNet game spawner.
Other
223 stars 86 forks source link

Refactoring MSBuild and Build Scripts #516

Closed frg2089 closed 3 months ago

frg2089 commented 4 months ago

For Generated CommonAssemblies.txt and CommonAssembliesNetFx.txt Run it on pwsh

.\scripts\build.ps1 -Games Ares -NoMove
.\scripts\Get-CommonAssemblyList.ps1 -Game Ares -Net8 > CommonAssemblies.txt
.\scripts\Get-CommonAssemblyList.ps1 -Game Ares > CommonAssembliesNetFx.txt
github-actions[bot] commented 3 months ago

Nightly build for this pull request:

SadPencil commented 3 months ago

@frg2089 I mark two previous conversation as unsolved again. You can write a bat file with powershell.exe.

https://stackoverflow.com/a/42898542 When using the powershell.exe command, note that the -executionpolicy parameter must be put before the -File parameter or it won't work.

C:\Users\WDAGUtilityAccount\Desktop>powershell -File helloworld.ps1
File C:\Users\WDAGUtilityAccount\Desktop\helloworld.ps1 cannot be loaded because running scripts is disabled
on this system. For more information, see about_Execution_Policies at
https:/go.microsoft.com/fwlink/?LinkID=135170.
    + CategoryInfo          : SecurityError: (:) [], ParentContainsErrorRecordException
    + FullyQualifiedErrorId : UnauthorizedAccess

C:\Users\WDAGUtilityAccount\Desktop>powershell -ExecutionPolicy Bypass -File helloworld.ps1
Hello, World!

So I have to write a bat script that uses pwsh to run it?

No. Use powershell. Or you can try pwsh first and use powershell as a fallback if you wish to. Use powershell decreases a dependency requirement.

Also, could be better if you places these bat files to the folder whose name is BuildScripts, back to the old time when we have not migrates to .NET 6

SadPencil commented 3 months ago

@frg2089 Also, good job on the common file hashes. If this script must require powershell v7 then you can dismiss my words about "use powershell instead of pwsh" things

frg2089 commented 3 months ago

@frg2089 I mark two previous conversation as unsolved again. You can write a bat file with powershell.exe.

@frg2089 Also, good job on the common file hashes. If this script must require powershell v7 then you can dismiss my words about "use powershell instead of pwsh" things

In fact, I don't want to go through the time it takes to get Windows PowerShell 5.1 compatible anymore. I'm spending too much time on build scripts. Since the people developing the client are C# programmers, they'd better get used to using PowerShell!

I haven't used more than the 5.1 syntax, which theoretically works on Windows PowerShell, but I haven't done any testing.

The only thing you need to do to fallback to powershell is to write something like this

@echo off
where pwsh > nul 2> nul
if %errorlevel% equ 0 (
  pwsh -ExecutionPolicy Bypass -File build.ps1 -Games Ares
) else (
  powershell -ExecutionPolicy Bypass -File build.ps1 -Games Ares
)

and changed the requires

#Requires -Version 7.2

to

#Requires -Version 5.1
SadPencil commented 3 months ago

Besides these two comments I don't have other complaints:

  1. I still didn't get it why there's an obsolete comment in csproj file

  2. If people double click the bat file, does the console automatically exits without showing the message? If so could be better to add a pause command so people can just double click the build scripts (.bat) like the old days

frg2089 commented 3 months ago
  1. I still didn't get it why there's an obsolete comment in csproj file

I occasionally engage in unfathomable behavior.

I don't want to break projects that used to use Directory.Build.Game.$(Game).props, but I've thrown compatibility out the window by rewriting the entire project's MSBuild file.

SadPencil commented 3 months ago

I mean the pause command out of the if scope. The build script might ends up with error messages.

Besides this change request I don't have more complaints

SadPencil commented 3 months ago

I need to additionally remind here that due to the current assembly loading order, we can only remove a dll from the common assemblies list; add a dll to the common assemblies list only if the dll is new, i.e., it has not been a specific assembly yet

Otherwise it would require removing old dlls in Binaries folder during the update

frg2089 commented 3 months ago

I need to additionally remind here that due to the current assembly loading order, we can only remove a dll from the common assemblies list; add a dll to the common assemblies list only if the dll is new, i.e., it has not been a specific assembly yet

Otherwise it would require removing old dlls in Binaries folder during the update

I think the order in which assemblies are loaded should be

  1. Special
  2. Common

Is that the way it is now?

SadPencil commented 3 months ago

I need to additionally remind here that due to the current assembly loading order, we can only remove a dll from the common assemblies list; add a dll to the common assemblies list only if the dll is new, i.e., it has not been a specific assembly yet Otherwise it would require removing old dlls in Binaries folder during the update

I think the order in which assemblies are loaded should be

1. Special

2. Common

Is that the way it is now?

Yes. Before .NET 4.8 downgrading it is common > special (ridiculous right?) and I've corrected to special > common

Metadorius commented 3 months ago

@frg2089 you can batch apply all the suggestions if you go to Files tab, that way they will be in one commit.

frg2089 commented 3 months ago

@frg2089 you can batch apply all the suggestions if you go to Files tab, that way they will be in one commit.

Sorry, I just noticed this.

Do you want me to clean up the commit log?

Metadorius commented 3 months ago

Sorry, I just noticed this.

No problems.

Do you want me to clean up the commit log?

No need, we will squash merge anyways.