Page MenuHomeWildfire Games

Debug version fix
ClosedPublic

Authored by Silier on Jan 8 2018, 9:33 PM.

Details

Summary

map starts now in debug version of game
sink rate for marker was too big for 15 bits

Test Plan

Build debug version
run it
start game

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Silier created this revision.Jan 8 2018, 9:33 PM
Owners added a subscriber: Restricted Owners Package.Jan 8 2018, 9:33 PM
Vulcan added a subscriber: Vulcan.Jan 8 2018, 10:12 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
temple added a subscriber: temple.Jan 8 2018, 10:31 PM

I think instead in Grid.h we want to ensure i1 <= m_W and j1 <= m_H?

	bool any_set_in_square(int i0, int j0, int i1, int j1) const
	{
	#if GRID_BOUNDS_DEBUG
		ENSURE(i0 >= 0 && j0 >= 0 && i1 < m_W && j1 < m_H);
	#endif
		for (int j = j0; j < j1; ++j)
		{
			int sum = 0;
			for (int i = i0; i < i1; ++i)
				sum += m_Data[j*m_W + i];
			if (sum > 0)
				return true;
		}
		return false;
	}
Silier added a subscriber: Itms.Jan 8 2018, 10:34 PM

[21:01] <@Itms> actually it should be min(..., m_W-1), good catch

Itms added a comment.Jan 8 2018, 10:57 PM
In D1209#49098, @Angen wrote:

[21:01] <@Itms> actually it should be min(..., m_W-1), good catch

For the record, I was wrong :)

wraitii requested changes to this revision.Jan 8 2018, 11:08 PM

I think temple is right here.

This revision now requires changes to proceed.Jan 8 2018, 11:08 PM
Silier planned changes to this revision.Jan 8 2018, 11:16 PM
Silier updated this revision to Diff 5180.Jan 9 2018, 10:06 AM
Silier edited the summary of this revision. (Show Details)
Silier edited the test plan for this revision. (Show Details)

editted ENSURE

Silier edited the summary of this revision. (Show Details)Jan 9 2018, 10:28 AM
This comment was removed by Silier.

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Silier updated this revision to Diff 5181.Jan 9 2018, 12:43 PM
Silier retitled this revision from Running debug version - Pathfinder borders fix to Debug version fix.
Silier edited the summary of this revision. (Show Details)
Silier edited the test plan for this revision. (Show Details)

In marker there was <SinkRate>1000000</SinkRate> but for integral part cannot be bigger than 16 bits

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
wraitii accepted this revision.Jan 10 2018, 8:24 AM

I'll commit this in two separate patches (if the first thing is indeed an issue, need to check how it gets parsed). Thanks for your work!

This revision is now accepted and ready to land.Jan 10 2018, 8:24 AM
Silier added a comment.EditedJan 10 2018, 8:53 AM

If you mean SinkRate problem:
Fixed.h uses to get double from string value of type CFixed_15_16 , what is 15 bits for integral part, and it
stays in integral part till . appears, anyway 10^6 > (2^3)^6 = 2^18 therefore it wants to use more than 15 bits and we get assertion error, what is handled somehow in
release build, but it is not good to have there such a thing.

Stan added a subscriber: Stan.Jan 10 2018, 9:14 AM

Have you checked it still sinks fast enough ?
Else you could go from 10 000 to 99 999 or whatever the upper bound is.

I do not see much difference between 10 000 and 30 000 animation, but maybe you can check visually if it helps.

Stan added a comment.Jan 10 2018, 1:40 PM

Okay since the point was to have a ridiculously high number setting it to the upper bound makes sense I guess.

This revision was automatically updated to reflect the committed changes.