[Dev] 0.1 "Babylon" Progress Thread

It needs msbuild and maybe some of the tooling, not the CLR. I'm a bit confused about what exactly the "Godot SDK" is, too, but I know we don't import the Godot namespace, and although the SDK is gettable with nuget I have been so far unsuccessful in trying to use any of Godot from outside Godot. Not that we need to, really...I just occasionally try to figure out if using xUnit is even possible Godot Mono code.

I'm not getting that. I'm wondering if that has to do with whatever you mean by going back and forth between dotnet and mono.

dotnet 5 has System.Text.Json "built in" whereas in mono 472 it needs to be added as a nuget package, but that's already in the csproj file, so I am not clear on why you would be seeing those errors.

Building with Godot should be performing a `nuget restore` before building which should fetch the needed nuget packages. I guess you can try that manually. The mono and dotnet tools put their build objects in different places I think, so if you are switching back and forth you'd need to "dotnet restore" for dotnet or "nuget restore" for mono, but I think the Godot build should be doing that for you.

Maybe try hitting that "build" button instead of build project? Quintillus found that it seems to do something extra over the build project or build scene buttons.

 

I tried doing the restore manually but that didn't work. I also tried adding the System.Text.Json package to the C7 project but that didn't work either. I was hoping adding the package manually (with "dotnet add...") would download a DLL or something but it all it did was add a line to the csproj file.

I guess the problem is that the Mono runtime that's part of Godot can't find packages that are part of .NET Core. I don't know why it can't install the missing package automatically, maybe it relies on the build tool to do that but that's not happening since the build tool thinks the package is already available as a built-in. I'll try installing System.Text.Json inside Godot's bundled Mono runtime (the "Godot SDK" ?). I searched the GodotSharp folder and as expected there's no System.Text.Json.dll, though there are Newtonsoft.Json, System.Runtime.Serialization.Json, System.Json, and System.Json.Microsoft DLLs.

Edit: Another thing I'll try is pointing Godot to the .NET Core libraries, after all System.Text.Json.dll must be somewhere on my system. Hopefully they're all compatible.

Tried it, no change.

 

Now that I try the build-in SAV (regenerated from Development), I can't get it to work or figure out why it's not working, either. (I've been using Civ3 saves recently, in part for more variety of the import to add things like volcanoes and to add support for maps with unequal width/height. Which means this may be my fault for not following the C7 SAV import method for the past couple weeks) I tried outputting the C7 save from the end of the Civ3 import process, so I could see what the differences were, but got the "Failure has occurred while loading a type" error there, as well. I haven't found the more detailed error that Flintlock was able to find.

Comparing the regenerated C7 save to the old one, the main structural difference is the addition of terrainTypes at the end of the gameData. I don't see anything wrong about the structure of that, though.

I'll see if I can get more info with a bisect. That should at least point us to a changeset that introduced the initial discrepancy.

Longer term, this is the sort of thing where if I knew C# testing as well as I know Java testing, I'd be adding an integration test for C7 SAV creation/re-opening once we get it working again. It probably is a sign that regardless, we should figure out how to get xUnit or whatever else is appropriate working, even if initially we're limited to parts of the repo. I've seen that Flintlock already has one imitation test in the code for lack of proper test suite integration, of the style I wrote in C before I figured out how to add proper tests in C. So there's some demand for proper tests.

I can't remember what it was that "build" fixed for me that not pressing that button didn't fix. Something about Godot not seeing changes.

 

Well, Git Bisect was helpful, it pointed me to the change to add support for Visual Studio 2019, and sure enough, it appears that one of the changes there resulted in not being able to load from the JSON. I have a PR open to revert that change (and update the JSON) here: https://github.com/C7-Game/Prototype/pull/66

Still investigating why that change caused an issue. It did add VS 2019 support, but I didn't think to try loading from JSON when I was checking out its changes a few days ago.

Edit: Confirmed the C7.csproj change to .netstandard 2.0, by itself, is sufficient to break the JSON handling; the .sln changes are not required for the JSON to stop working. Also confirmed the .sln changes do not break JSON handling, so I added those back in.

 

All right, the PR should be reduced to a minimal required set of changes.

I've requested review from @Puppeteer and @Flintlock to confirm that these changes fix the JSON loading from them, and from @Lanzelot to confirm that the Visual Studio workflow still works for him.

Slightly longer-term, we probably do want .netstandard2.0 for C7.csproj, but with JSON support still working. For now I think it's better to fix JSON first though.

 

Ok I haven't been paying very close attention lately and didn't realize C7.csproj went to netstandard2.0 . I'm a bit surprised that works, but I guess it kinda makes sense. I thought our ideal was to converge on net472 for everything, though, and the netstandard2.0 was my own personal kludge because I didn't know how to make utility console apps with mono. (Which I think I finally have figured out and have a PR #65 to make _Console/BuildDevSave use mono on the console.)

Oh, and I just remembered that xUnit does NOT work with netstandard2.0 . Which meshes with my next point that I have yet to figure out how code with `using Godot` can be unit tested, but the non-Godot-tied libraries should be testable with xUnit if we change them to net472 (or anything but one of the netstandards). And the load/save functions should be testable this way right now.

 

Incidentally, I should be sure to point out I am pretty convinced that Flintlock's System.Text.Json errors and the rest of us crashing on loading the json save are two entirely different issues, even if they seem to present in a similar way. Maybe the VS changes are causing both, but they are two different problem paths.

I'm poking in an `xUnit` branch now (not yet pushed) where I put everything as net472 (except Blast, the external library), and I'll see if the save read crashes without Godot which at this moment I'm presuming it will.

 

Ok, that's not what I expected, but I'll take it: Changing all (except Blast which is not ours) netstandard2.0 to net472 makes "new game" work for me. I just pushed the xUnit branch. Try the all-net472 tag for the actual commit as I might poke at actual unit testing later.

Tagging @Quintillus and @Flintlock as you may be looking at this now.

 

I just checkout Development again and changed *only* C7.csproj to net472. That also works for me. (I'm not going to push that or make a branch; just letting you know.)

The main project shouldn't be netstandard, but libraries can be to share between other projects, generally speaking. I don't really understand what's going on here, but it sort of meshes with my vague understanding of .NET so it makes enough sense to me that the main project should not be a netstandard.

I am personally ready to fully commit to making everything net472, and I feel like I caused all this confusion in the first place. But I don't have full VS and don't know what will work with it and what won't.

Edit: As a testing note, I think it would have taken an integration test and not just a unit test to catch this issue. On the other hand, having C7GameData, C7Engine, and the other libraries unit tested would have at least narrowed down the scope of the problem.

Edit 2: Oh, changing the C7 target framework is exactly what Quintillus' PR does. I reviewed/approved the PR but do not have VS to test against it.

 

Here I was thinking .NET 4.7.2 was the temporary one, and that we planned to have .netstandard 2.0 be the standard one. Looking over the Godot / Mono Synchronization thread, I see that you wrote, "the only reason the non-Godot-using libraries are dotnetstandard2.0 is because of my inability so far to grok or even know about Mono command line tools", but from my next post it's clear that at the time I considered them pretty much interchangeable, which is why the .csproj change didn't set off a lightbulb when I saw it.

Probably a silly question, but any reason you plan to standardize on 4.7.2 instead of 4.8? I've used both of those interchangeably on the static JSON project, and it seemed to work there, but am not sure if it will everywhere yet. Looking through the release announcement, I don't see any feature removals mentioned, and 4.8 still supports Windows 7 SP1, so it looks like it should generally be okay.

For future C# versions, it appears that Godot is going to stick with .NET 4.x for Godot 4.0, and add support for .NET 6.0 in Godot 4.1. So that answers the question of whether we should move to the "new" .NET.

 

When you make a new Godot Mono 3.4 project, it makes it net472, and I don't know what the impact of changing that would be. I figure our target version is tied to our Godot version which is providing the runtime.

Actually I guess I need to double-check that assertion as we created this project with a pre-3.4 Godot.... Yep, it still makes a new project with net472.

 

That sounds like a good enough reason to stick with net472 to me. If we find out there's some super cool feature in 4.8 that would make life easier, we can revisit it then.

I left a comment on the readme of @Puppeteer 's update. Questionable whether I'll get to the much larger one from Flintlock before Christmas. Yes, I'm one of those developers who sees a 30-line PR and knocks it out in 3 minutes, but is less eager about an 850-line one that may well take 85 minutes.

 

Surprisingly that FixC7SAVLoading branch fixes "new game" for me too. I didn't expect that, I thought the problem was due to differences between the Mono and .NET SDKs. Speaking of which, you guys never replied to my question about which one you are using. Am I correct in assuming you're all using the Mono SDK?



Edit:

This is good to know. I was planning to merge the PR in any day now, figuring you guys had had a chance to see it even though you never commented on it specifically.

 

I have both installed. I don't know what Godot is using. VSCode seems to be using external Mono for Intellisense.

I don't think the version installed matters; I think Godot just needs one of them for msbuild and maybe another tool or two. ... Huh, actually it seems to specifically reqire ".NET Core" which should be styled dotnetcore but is just dotnet as of v5.

https://docs.godotengine.org/en/stable/getting_started/scripting/c_sharp/c_sharp_basics.html

Edit: This adds a lot of possible confusion back into the console utility platform question. Whee.

Edit 2: If I'm reading this right, we're using dotnetcore v3.1 or dotnet v5 to compile code that is run by Mono emulating .NET Framework 4.7.2. Oh, and the csproj specifies Godot SDK. WTF Microsoft?

Edit 3: Ok, apparently ".NET Core" is the correct styling after all. Not sure if that changed or if I've always been wrong.

 

I'm not sure how to check which version Godot is using, either. I mean, it's the Mono version of Godot, but I've used the traditional Windows installers for installing all the .NET runtimes, which would seem to imply I'm not really using Mono. How to figure that out? I have no idea.

Yeah, I will comment if I have reviewed a PR. But between present-acquiring, family-visiting, friend-socializing, and rona-testing (negative), there haven't been a lot of unallocated 90-minute blocks when I've had the energy to be productive and focused. Looks like that's case for everyone else, too.

 

You can check which one Godot is set to use if you go into the editor settings then look at the "Builds" section, which is under "Mono" at the bottom of the list. "dotnet CLI" means it's using .NET Core and "MSBuild (Mono)" means it's using Mono SDK.

 

Mine says "MSBuild (Mono)" which conflicts with what the site I linked earlier says.

But the msbuild on my path does say it's for Mono which at least comforts me a little. So I guess I'm using Mono?

 

Mine says "dotnet CLI".

Which is different than what Puppeteer's says, which discomforts me a little.

 

I just installed Visual Studio 2019 on Windows to see what's up. I'm not sure what @Lanzelot is doing with it, but if I choose build it succeeds with 11 warnings. If I go into Godot and build from there it still works. This is with the latest merge of PR#66 a few minutes ago with net472 in the csproj file.

This is a new install of VS 2019 community with a bunch of the .NET stuff checked for install including the .NET 4.7.2 SDK.

 

I've reviewed about 25% of Flintlock's animation PR. Not going to get the rest done tonight or probably till some time next week. Maybe not till 2022 if next week winds up being a socially active week, which is a distinct possibility.

I do wonder if the PR process is slowing things down too much. We have 5 open currently, although Puppeteer's is approved. I definitely feel like I'm not writing much code anymore and am mostly the reviewer. Which isn't all bad; I am making some suggestions (it'd be boring to review PRs and not make any suggestions). But I'm not sure if it's really an efficient use of time.