fire-eggs / CivOne

An open source implementation of Sid Meier's Civilization 1 using C# and .NET.
Creative Commons Zero v1.0 Universal
21 stars 4 forks source link

Unit tests breaking: initial unit placed on Ocean tile #139

Closed iegik closed 3 years ago

iegik commented 3 years ago
            Assert.True(tile is Grassland);
           ^
Exception has occurred: CLR/Xunit.Sdk.TrueException
An exception of type 'Xunit.Sdk.TrueException' occurred in xunit.assert.dll but was not handled in user code: 'Assert.True() Failure'
   at Xunit.Assert.True(Nullable`1 condition, String userMessage) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\BooleanAsserts.cs:line 96
   at Xunit.Assert.True(Boolean condition) in C:\Dev\xunit\xunit\src\xunit.assert\Asserts\BooleanAsserts.cs:line 63
   at CivOne.UnitTests.IrrigateTest.AllowIrrigate() in xunit/src/IrrigateTest.cs:line 26

(need more info)

fire-eggs commented 3 years ago

The game generator has placed the Chinese settler unit on an Ocean tile!

The unit tests are run with a hard-coded random seed [see TestsBase2 ctor] which "should" produce consistent results.

So I see two problems here:

  1. The map/game generator is not producing the same results for a given random seed [as compared to Oct 29, 2019].
  2. The game generator is placing the initial civilization settler on an Ocean tile and not correcting it.

Issue 2 here needs to be fixed first: The Game generator is in Game.NewGame.cs ; there is a function called AddStartingUnits which would be the place to look at.

fire-eggs commented 3 years ago

The tests ZOCFixTest1 and ZOCFixTest2 are misbehaving for similar reasons : the map generator has produced a different map than when the tests were written.

Each of those tests call a method named TestZoc in which a Chariot is created on the map at coordinates [53,16]. That tile is now an Ocean tile.

ZOCFixTest1 "succeeds" but should not as the Chariot is in an invalid location. This is a failure of TestZoc to validate the Chariot is correctly placed. ZOCFixTest2 "fails" and should not.

Similar problems occur for the ZOCRevertTest* tests.

iegik commented 3 years ago

I think, that "Earth" map would not exist without these needs. So, can we run all our tests on Earth map with same seed for resource generation?

Off-topic: Can we represent tiles with units on it in bytes? Or how they actually represents (3x3 tile examples:)

terrain   resource  unit
01 02 03  00 00 08  00 00 00
01 02 03  00 00 00  00 01 00
01 01 02  07 00 00  00 00 00

With that 0..255

fire-eggs commented 3 years ago

Off-topic: Can we represent tiles with units on it in bytes?

How do you handle stacked units?

So, can we run all our tests on Earth map with same seed for resource generation?

That was the intent. If done correctly, a given seed value should create the same landscape, select the same civilizations, place the initial civilization settlers at the same locations, etc.

I added a command-line parameter seed [runtime\sdl\src\Programcs]. So by setting the Visual Studio debug parameter to (e.g.) "-seed 12345" the game "should" be the same each time. Whether that is still the case needs to be verified.

fire-eggs commented 3 years ago

There was a problem with the Runtime mock so that it could not load the Earth map. When the Earth map cannot be loaded, all tiles are set to Ocean.