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

chore: update .NET Framework version #2

Open
wants to merge 10 commits into
base: feature_ci
Choose a base branch
from

Conversation

steveski
Copy link

@steveski steveski commented Jun 22, 2022

  • Updated to .NET 4.7.1
  • Replace appveyor ci and cd with GitHub Actions
    • GitVersion in place to enable semantic build version numbers
    • ci.yml produces zip files with beta in the filename when merge to main occurs
    • ci.yml produces zip files with pullrequest in the filename when building against a PR
    • ci.yml produces zip files with branch name in the filename when a push is made to a branch where the no PR
    • cd.yml will attach a GitHub release zip to the repository when you push a semantic version tag

I use deepest scope for namespacing. There's a minor potential vulnerability having using statements outside the namespace declaration. Probably not an issue in practice, but something I like to do.

I got a little stuck on the GuidGenerator getting it to be compatible with the other .NET versions of dependencies being link into the netstandard server side projects. Spent a little time looking at it but got stuck. How important is it to have the custom generator?

In the .NET 4.7.1 projects like this one there isn't so much of a difference but in the server projects where they target netstandard the differences become alot more obvious due to the differences given .NET Core is in the mix.

Plugins/Name.cs Outdated Show resolved Hide resolved
Plugins/Repository.cs Outdated Show resolved Hide resolved
NFive.SDK.Core.nuspec Outdated Show resolved Hide resolved
@JoeBiellik
Copy link
Member

Regarding the custom GUID generator, its not vital but does significantly speed up MySQL using the GUID as a primary key, there are plenty of articles talking about the performance impact of using UUID/GUIDs in SQL indexes. Your change from new RNGCryptoServiceProvider() to RandomNumberGenerator.Create() should be all that needs to change in order to work under .NET Core. Does this cause issues in FiveM?

Comment on lines -4 to -9
[assembly: AssemblyTitle("NFive Core SDK")]
[assembly: AssemblyDescription("NFive core SDK for plugin development")]
[assembly: AssemblyConfiguration("")]
[assembly: AssemblyCompany("NFive")]
[assembly: AssemblyProduct("NFive SDK")]
[assembly: AssemblyCopyright("Copyright © NFive 2018-2021")]
Copy link
Member

Choose a reason for hiding this comment

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

Need to move the relevant fields here to the csproj file as these are used in NFive.SDK.Core.nuspec, reference here,

@steveski
Copy link
Author

steveski commented Jul 1, 2022

Regarding the custom GUID generator, its not vital but does significantly speed up MySQL using the GUID as a primary key, there are plenty of articles talking about the performance impact of using UUID/GUIDs in SQL indexes. Your change from new RNGCryptoServiceProvider() to RandomNumberGenerator.Create() should be all that needs to change in order to work under .NET Core. Does this cause issues in FiveM?

Not an issue running in FiveM, but I found that the RNGCrypto one prevented other libs linkig to this one from being able to link due to .NET version incompatibilities.

@all-in-simplicity
Copy link

all-in-simplicity commented Jul 5, 2022

Regarding the custom GUID generator, its not vital but does significantly speed up MySQL using the GUID as a primary key, there are plenty of articles talking about the performance impact of using UUID/GUIDs in SQL indexes. Your change from new RNGCryptoServiceProvider() to RandomNumberGenerator.Create() should be all that needs to change in order to work under .NET Core. Does this cause issues in FiveM?

RandomNumberGenerator.Create() causes following runtime error in FiveM:

^1SCRIPT ERROR in task continuation: System.MethodAccessException: Error verifying NFive.SDK.Core.Helpers.GuidGenerator:GenerateTimeBasedGuid (): Method System.Security.Cryptography.RandomNumberGenerator:Create () is not accessible at 0x0009^7

I think it would be better to stick with the original implementation of the GUID generator.

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