Power jumps to 100 000 in campaign beta, mission 1

[wzdev-ci]

keyword_power resolution_fixed type_bug | by Emdek

---

After around half of mission time power level jumped to 100 000 from 0 (some building were built and vehicles manufactured).
I can attach saves if needed. As far as I remember it occurred soon after saving game.

Very nice bug by the way. ;-)

---

Issue migrated from trac:1581 at 2022-04-15 21:05:33 -0700

[wzdev-ci]

Per commented

---

If you have a savegame where it reproduces the problem, please attach it and tell us how to do it. Thanks.

[wzdev-ci]

Emdek uploaded file savegames.tar.bz2 (300.5 KiB)

One save game before and one after power jump

[wzdev-ci]

Emdek commented

---

I don't know what I did. ;-)
I've saved game and noticed that structures are build at high speed etc.
Maybe this is something with saving games?

[wzdev-ci]

hao commented

---

I've seen this too, but i can't provide any further info. I even think that it happened on one of the further levels (around the middle of beta campaign).

[wzdev-ci]

Per commented

---

@Emdek: "structures are build at high speed etc." Did game speed increased massively for a short while?

[wzdev-ci]

Emdek commented

---

Replying to Warzone2100/old-trac-import#1581 (comment:4):

@Emdek: "structures are build at high speed etc." Did game speed increased massively for a short while?
Nope. They only were built at maximum build speed.
But in case of game speed, it is possible that I've changed speed from normal, but don't remember if increased or more probably decreased.

Now I've got similar thing in last gamma mission, or it is normal that we start it with full power?

[wzdev-ci]

Per changed status from new to closed

[wzdev-ci]

Per set resolution to fixed

[wzdev-ci]

Per commented

---

(In [9897]) 2.3: Add mitigation in case an underflowed value is passed into addPower(). May fix #1581

[wzdev-ci]

Emdek changed status from closed to reopened

[wzdev-ci]

Emdek changed resolution from fixed to ``

[wzdev-ci]

Emdek commented

---

This still happens in beta11a.
This time in mission 3, campaign beta.
Game speed: 0.8.
I don't know if power amount was around 0 this time (was around 800 and then I've started building some vehicles).

[wzdev-ci]

Safety0ff uploaded file moreDebugOptions.diff (2.2 KiB)

Uploaded incase this is useful to someone, use at your own peril.

[wzdev-ci]

Emdek commented

---

Also happened once in gamma 2 mission...

[wzdev-ci]

Emdek uploaded file warzone2100.gdmp-8HoxKP (18.9 KiB)

Dump created after adding ASSERT(power < MAX_POWER, "Underflow attempt detected"); to power.c as discussed on IRC.

[wzdev-ci]

Safety0ff uploaded file powerassert.diff (0.9 KiB)

[wzdev-ci]

Safety0ff changed milestone from `` to 2.3.1

[wzdev-ci]

anonymous uploaded file b01.tar.gz (185.8 KiB)

[wzdev-ci]

Buginator commented

---

Ok, I'll bite. What do those savegames show? Everything is working normally from what I see.

Care to give more detailed steps on how to replicate the issue?

[wzdev-ci]

Emdek uploaded file warzone2100.gdmp-useqe7 (18.9 KiB)

New dump, using new assertion patch.

[wzdev-ci]

Emdek commented

---

Replying to Warzone2100/old-trac-import#1581 (comment:10):

Ok, I'll bite. What do those savegames show? Everything is working normally from what I see.

Care to give more detailed steps on how to replicate the issue?
SafetyOff suggested that this could be 32 bit issue only, I can reproduce it always (sooner or later) using my 32 bit Mandriva 2010.1 installation.
For asserts I'm reproducing it this way:

1. load my savegame;
2. order factories to build heavy cannons in infinite loop;
3. order trucks to build some expensive structures (for example hard points in line);
4. order research facilities to research expensive topics;
5. increase game speed to 1.5 (I'm not sure if it's needed, but waiting time for results is shorter, and as far as I remember it happens also with other speeds);
6. wait for crash (if assertion patch applied), this time it took around twenty "game minutes".

[wzdev-ci]

Safety0ff changed status from reopened to assigned

[wzdev-ci]

Safety0ff set owner to Zarel

[wzdev-ci]

Safety0ff commented

---

Looks like it was introduced in [9172] (circa beta 8.)

I'll let Zarel fix this.

[wzdev-ci]

Safety0ff commented

---

Replying to Warzone2100/old-trac-import#1581 (comment:11):

Safety0ff suggested that this could be 32 bit issue only, I can reproduce it always (sooner or later) using my 32 bit Mandriva 2010.1 installation.
I remember asking about your platform, if I did say that then it wouldn't be likely to be true since WZ uses fixed size integers (for the most part.)

[wzdev-ci]

Zarel changed status from assigned to closed

[wzdev-ci]

Zarel changed resolution from `` to fixed

[wzdev-ci]

Zarel commented

---

(In [10769]) Fix bug #1581 - incorrect power calculation

[wzdev-ci]

Zarel commented

---

(In [10770]) 2.3: Fix bug #1581 - incorrect power calculation

[wzdev-ci]

Git SVN Gateway <gateway@...> commented

---

(In Warzone2100/warzone2100@2197ffe) Fix bug #1581 - incorrect power calculation

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@10769 4a71c877-e1ca-e34f-864e-861f7616d084

[wzdev-ci]

Per changed resolution from fixed to ``

[wzdev-ci]

Per commented

---

The problem is not gone. It is much worse. Just start a research that you do not have the power to pay fully, and you will immediately be granted 100 000 power. This is in 2.3/branch.

[wzdev-ci]

cybersphinx changed priority from major to blocker

[wzdev-ci]

cybersphinx commented

---

I removed the power ceiling hack and ran it through the debugger. At some point, all players get ~42950000 added to their power - I think that's 2^32 / 100.

[wzdev-ci]

cybersphinx uploaded file test.7z (55.3 KiB)

Savegame to reproduce, just wait a few seconds.

[wzdev-ci]

cybersphinx commented

---

To be more specific about "some point", it is when gameTime % (1<<16) is smaller than pResExtractor->timeLastUpdated % (1<<16) while extracting resources - and that can happen every 65536 ms. WTF was this modulo thing supposed to solve?

[wzdev-ci]

cybersphinx uploaded file 0001-Simplify-power-calculation-in-updateExtractedPower()..patch (1.2 KiB)

Proposed fix. I have no idea what the modulo was supposed to solve, but my version should always work, since timeLastUpdated should never be larger than gameTime.

[wzdev-ci]

cybersphinx uploaded file 0001-Simplify-power-calculation-in-updateExtractedPower.patch (1.8 KiB)

Oops, this one should actually work.

[wzdev-ci]

Crymson commented

---

I was about to file a bug report on this.
I think Zarel was on drugs when he wrote that fix.

[wzdev-ci]

Zarel commented

---

Your fix has rounding problems and the like.

I will provide a proper fix sometime today.

[wzdev-ci]

Zarel commented

---

I mean, your patch disregards

// Written this way to prevent rounding errors - do not "simplify"

It also reintroduces the problem of an uninitialized extractor (the "power level jumps to 100,000 when moving from cam1 to cam2 or cam2 to cam3" error).

[wzdev-ci]

cybersphinx commented

---

Replying to [comment:25 Zarel]:

I mean, your patch disregards

// Written this way to prevent rounding errors - do not "simplify"

Why? Previous version is aX - bX, new version is (a - b)*X. Should be exactly the same. I guess it loses some decimals due to the integer precision, but I don't see how your old version prevented that.

It also reintroduces the problem of an uninitialized extractor (the "power level jumps to 100,000 when moving from cam1 to cam2 or cam2 to cam3" error).

If the extractor is uninitialized, how about actually initializing it on campaign change? If your version prevents the problem, it is completely unobvious, so better comments would be appreciated.

[wzdev-ci]

Zarel commented

---

Replying to [comment:26 cybersphinx]:

Why? Previous version is aX - bX, new version is (a - b)*X. Should be exactly the same. I guess it loses some decimals due to the integer precision, but I don't see how your old version prevented that.

Which means cumulative of (a*X/Y - b*X/Y) + (b*X/Y - c*X/Y) would be a*X/Y - c*X/Y, i.e. no loss to rounding at all.

On the other hand, your cumulative is (a - b)*X/Y + (b - c)*X/Y, which means error could be up to Y-1 and on average would be Y for each frame.

Keep in mind that Y is GAME_TICKS_PER_SEC * 100, which is 100,000 per frame. Also keep in mind that this is calculated separately per extractor, so for 15 extractors (an average game), that's accumulation of rounding error 900 times per second.

A significant drop-off, enough that some of the power upgrade tech could have practically no effect if you got unlucky with rounding, which is why I rewrote it in the first time.

If the extractor is uninitialized, how about actually initializing it on campaign change? If your version prevents the problem, it is completely unobvious, so better comments would be appreciated.

Well, that's also a good solution, but you didn't do that.

You did, however, remove:

// if the extractor hasn't been updated recently, now would be a good time. 
if (pResExtractor->timeLastUpdated < 20 && gameTime >= 20) 
{ 
    pResExtractor->timeLastUpdated = gameTime; 
}

which was my (admittedly fairly hacky) fix for the problem.

[wzdev-ci]

cybersphinx commented

---

Replying to [comment:27 Zarel]:

Which means cumulative of (a*X/Y - b*X/Y) + (b*X/Y - c*X/Y) would be a*X/Y - c*X/Y, i.e. no loss to rounding at all.

But what actually happens is not what you wrote, but int(aX - bX) + int(bX - cX). You just lose less digits. For a real solution, you'd need to keep the fractional part somewhere (maybe by setting timeLastUpdated not to gameTime, but gameTime - (the time it takes for the rest to accumulate)).

Well, that's also a good solution, but you didn't do that.

Right. I was just poking around trying to remove that modulo crap (since I have no idea about the original problem).

[wzdev-ci]

Zarel changed status from reopened to closed

[wzdev-ci]

Zarel changed resolution from `` to fixed

[wzdev-ci]

Zarel commented

---

(In [10885]) 2.3: Third try to fix bug #1581 - problems in oil derrick code.

[wzdev-ci]

Crymson changed status from closed to reopened

[wzdev-ci]

Crymson changed resolution from fixed to ``

[wzdev-ci]

Crymson commented

---

Nope, still not fixed.
In a MP game, team mate had no power and I transferred some to him, and then he got 100K.

[wzdev-ci]

Zarel commented

---

Replying to [comment:30 Crymson]:

Nope, still not fixed.
In a MP game, team mate had no power and I transferred some to him, and then he got 100K.

Were you playing 2.3.0 or svn/2.3 [10885]+?

[wzdev-ci]

Safety0ff commented

---

Replying to [comment:30 Crymson]:
Nope, still not fixed.
In a MP game, team mate had no power and I transferred some to him, and then he got 100K.
Almost sounds like a different bug.

[wzdev-ci]

Zarel changed status from reopened to closed

[wzdev-ci]

Zarel changed resolution from `` to fixed

[wzdev-ci]

Zarel commented

---

In fact, I'm pretty sure it is. Please create a new bug report.

[wzdev-ci]

cybersphinx commented

---

Replying to [comment:30 Crymson]:

Nope, still not fixed.
In a MP game, team mate had no power and I transferred some to him, and then he got 100K.

I just tried to reproduce that with current svn, and it seemed to work correctly.

Anyway, I guess a new ticket would be nice, since this one is getting long.

[wzdev-ci]

Zarel commented

---

Two things of relevance:

1. Both these situations deal with addPower (The beginning of CAM_3B has a scripted event that adds 5K power), while my 2.3.x power changes deal with the unrelated updatePlayerPower/updateCurrentPower/updateExtractedPower loop.

2. Power jumps to ~4 billion were reported in 2.1.x and 2.2.x well before I implemented the 2.3.x power calculation changes.

Because of this, I have reason to believe the bug originally reported by Crymson is unrelated to any recent changes we have made, and so I stand by my closing of this ticket. Why hasn't a new ticket been made for this bug?

[wzdev-ci]

buginator <buginator@...> commented

---

(In Warzone2100/warzone2100@6421c81) CHANGELOG: Fix rearming pads to actually repair units

refs #1581
fixes #2313
fixes #2234

Fix rearming pads so they actually rearm at the speed they're intended to. Also make rearming upgrades affect rearming pad repair speed. Replaces REARM_PAD.currentPtsAdded with REARM_PAD.timeLastUpdated, but savefiles should still be compatible.

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@8865 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 75569d156f708b460609f50be5da4c2dbf896cfa)

Conflicts:

src/game.c
src/structure.c

[wzdev-ci]

buginator <buginator@...> commented

---

In Warzone2100/warzone2100@6421c81:

#CommitTicketReference repository="" revision="6421c81d7958e67334e2a3cfa190f24b40e9eb02"
CHANGELOG: Fix rearming pads to actually repair units

refs #1581
fixes #2313
fixes #2234


Fix rearming pads so they actually rearm at the speed they're intended to. Also make rearming upgrades affect rearming pad repair speed. Replaces REARM_PAD.currentPtsAdded with REARM_PAD.timeLastUpdated, but savefiles should still be compatible.

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@8865 4a71c877-e1ca-e34f-864e-861f7616d084
(cherry picked from commit 75569d156f708b460609f50be5da4c2dbf896cfa)

Conflicts:

	src/game.c
	src/structure.c