Fix AddCargo for py3

[P-D-E]

Cargo missions in campaigns (e.g. Jenek's) give an error when the cargo is loaded on the ship but there's not enough space for it. @stephengtuggy reported it in the issue below, and the error is TypeError: 'type' object is not subscriptable cause by the line

rang=list[list(range(you.numCargo()))]

On my Linux system with Python 3.9.7 the error is different TypeError: unbound method list.reverse() needs an argument and it happens on the next line

rang.reverse()

Due to this difference in behavior, pre-emptive test of this fix on all platforms is a must.

Thank you for submitting a pull request and becoming a contributor to Vega Strike: Upon the Coldest Sea.

Please answer the following:

Code Changes:

• [x] Have the PR Validation Tests been run? See https://github.com/vegastrike/Vega-Strike-Engine-Source/wiki/Pull-Request-Validation
• [ ] This is a documentation change only

Issues:

• Please list any related issues https://github.com/vegastrike/Assets-Masters/issues/32

Purpose:

• What is this pull request trying to do? Fix the error, allowing the original behavior: some cargo gets sold automatically to make room for the mission cargo.
• What release is this for? 0.8.x
• Is there a project or milestone we should apply this to? 0.8.x

[stephengtuggy]

Tested this just now. It didn't seem to work quite right. I ended up with 2500 m3 of cargo in my starting Llama's 2000 m3 hold. 😆

[P-D-E]

@stephengtuggy Weird, on my system both the old solution and the newer one by @BenjamenMeyer work as intended. Could you activate debug in your config and see what you get in the logs from the lines between 358 and 388?

[stephengtuggy]

@P-D-E The cargo I tested with was Astral Gases, which take up a lot of volume, but have very little mass. That might make a difference in the results of the test.

[stephengtuggy]

I filled up my hold completely with Astral Gases. Then accepted Jenek's second mission. Which should have auto-sold 500 cubic meters of Astral Gases, but didn't.

[stephengtuggy]

Am I understanding that correctly?

[P-D-E]

@stephengtuggy Yes, the sense of the reversed range is probably to start selling from the latest purchase, and break the loop as soon as there's enough space for the mission cargo. The debug lines document the logic in action; besides activating debug, you could turn them into prints and keep an eye on the terminal.

The tests I had run involved solid goods, and also of different kinds, and it worked as expected; you could try like this and I see what happens with Astral Gases.

[P-D-E]

Ok here's a quick update: tested with a Lancelot (75 cargo units available) on Jenek's 1st mission (2 cargo units required).

1. Bought 3 Astral Gases units, 25x3, 0 space left. Mission accepted, result: -2 units available, I had all the gas AND the mission cargo.
2. Bought 2 Astral Gases units and 1 Atmospheric Gases unit, different price but same specs so again 25x3, 0 space left. Mission accepted, result: Atmospheric Gases were sold, and I had the 2 Astral Gases, the mission cargo, and 23 units free.

So it seems that it's not the kind of cargo per se, but having only one kind of cargo filling the whole hold. To avoid doubt about the peculiar nature of gases, I restarted buying 75 units of Recycled Electronics, 75x1, 0 space left. Mission accepted, result: -2 units available, I had all the electronics AND the mission cargo.

This seems to confirm the bug with one kind of cargo filling the whole hold.

[stephengtuggy]

@P-D-E Sounds like you've narrowed it down pretty well. Is it a boundary condition? An off-by-one error on one of the bounds of that for range loop, perhaps?

[P-D-E]

@stephengtuggy Yes that's the main suspect, will check if original code had the same bug first of all, then proceed with investigations.

[P-D-E]

The original fix, rang=list(range(you.numCargo())) followed by rang.reverse(), works; it sells all the electronics, not just a small amount, the auto-sale part doesn't go in detail. Off-by-one confirmed, Ben's range includes the upper limit and excludes the lower (e.g. 8 to 1 instead of 7 to 0) hence it skips the oldest cargo. A quick fix would be for i in range(you.numCargo() - 1, -1, -1):

[stephengtuggy]

I will want to test this again, but it sounds like it's fixed. 🎉

[P-D-E]

@stephengtuggy

I will want to test this again,

Yes please, I'm only building/testing on Linux.

but it sounds like it's fixed. tada

Hopefully!

[stephengtuggy]

Tested and working on Windows. I accepted Jenek's 1st mission after loading a Pacifier full of Astral Gases. After I completed that mission, I bought a Lancelot at Ataraxia Fighter Base; flew it to Atlantis; loaded it to capacity with Astral Gases (3 units); and then accepted Jenek's 2nd mission. All went well, except that Serenity appeared to have disappeared without a trace by the time I was ready to head there on the second mission. Not sure what that was about. I assume that it was a random occurrence, where the base got blown up by some capital ship or other. At any rate, the cargo- and mission-handling logic worked exactly as I expected.