Periodic Housekeeping PR 1

[royfalk]

• Remove VS_PI and replace with standard M_PI
• Fuel tank is integrated (can't upgrade). Remove options in base_computer.
• Use standard ftl_energy.MaxLevel() instead of getWarpEnergy and warpCapData.
• Remove support for additive and multiplicative capacitors and reactor.
• Make unit_csv load reactor and capacitors the standard way using Load.
• Remove WCfuelhack in unit_csv. Make energy source a configuration item. Game doesn't need to compute this every loop.
• Remove vsfilesystem reference in unit_csv_factory. Breaks testing.
• Remove reference to Unit in Cloak. Improves encapsulation.
• Merge EnergyType enum into ComponentType.
• Move GetUpgradeType to component_utils.
• Remove Component::Describe. We'll do this in python along with the rest of the text in base_computer.
• Make component query manifest for price, volume and description.
• Add a whole bunch of getters to component.
• Add support for infinite energy source. Useful for WC drive and reactor.
• Move factors to configuration. You can now control utilization of drive and other components using the config file.
• Add resiliency to Graphics2 and Manifest loading. Does not crash game if missing.

Code Changes:

• 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:

• Jump causes damage. Unclear if this is new. There is code to do this in the engine.
• Python ship view doesn't include the factors. Specifically, FTL drive factor (0.1) isn't applied in python.

[evertvorster]

I'm having a little trouble making this as a patch that applies to master.
I might have to try a different way of getting this source to compile.

--edit... just checked out the branch you are working on, and it is compiling now. ;)

[evertvorster]

OK, first mayor problem.
When starting a new campaign, the SPEC drive seems to draw energy from the drives, and depletes the drive capacitor in a second, and then disengages.
This is true for both the initial Llama, and the Llama you get when you do the first repair and refuel.
Suffice it to say, the game is unplayable without SPEC.

When loading a saved game with a different ship, the SPEC drive seems to perform normally.

Also, if you start a new campaign, and immediately save your game and then exit VegaStrike, loading that saved game on the next run coredumps VegaStrike.

terminate called after throwing an instance of 'std::invalid_argument'
  what():  stod
/usr/bin/vs: line 28: 43001 Aborted                 (core dumped) vegastrike -d"${VEGASTRIKE_SHARE_DIR}" "$1"

[evertvorster]

Some added information, I checked with master branch, and on that SPEC works normally.

[royfalk]

The crash is because of #865 . It seems I didn't fix the issue. I had a recollection that I did.
I'll fold this into vegastrike/Assets-Production#128.

[BenjamenMeyer]

we probably should just do whole credits and not concern ourselves with fractional credits
(e.g 1 credit = 1 penny = 1 shilling .... )

[royfalk]

Yeah. I figured that one out too. Still need a todo.

[evertvorster]

Just did a little cross-checking here.

I'm pretty sure this PR introduces #898 as the issue is not present in the current master.
Is this what that extra python script is about?

Also, please update this branch by merging in master.

[royfalk]

Also, please update this branch by merging in master.

I will.

I find myself with a bit more time and to facilitate reviews by @BenjamenMeyer , I'm working on multiple branches in parallel. This is all fine and dandy, except when it isn't.

[royfalk]

When starting a new campaign, the SPEC drive seems to draw energy from the drives, and depletes the drive capacitor in a second, and then disengages.

I think I fixed this in vegastrike/Assets-Production#129. You can test this by making the changes to ships.json.

[evertvorster]

Hi there.
Yes, the multiple branches are a little confusing to me too. What is most confusing is that there seems to be some inter dependency between the branches, and definitely between branches between Assets and Engine.

Of course, a little testing can help to get things merged faster and then hopefully we will have less branches hanging around.

I'm testing this branch now. It's running together with task_ship_view branch of Assets that has been patched with PR 129.

So far, its pretty good. I am able to start a new campaign, and save the game. When starting a new instance of vs and loading the created saved game, you now have flight controls, which is great.
Unfortunately you can't buy a jump drive and SPEC does not stay engaged, but now it does not show as a drain on the drives bar, it just kicks on and then disengages. This sounds awfully familiar, and I'll go check to see if I am missing a patch or something.

[evertvorster]

Ah, I see what the issue is. In #896, the issue with SPEC and buying the jump drive is fixed, but not this branch.
I'll go test as much as I can of this branch so that it can get merged, and then we'll concentrate on getting #896 with a view of getting that one merged next.

[evertvorster]

Just saw my ship take some damage going through a jump as well, that's definitely something new.
For reference, I was flying a Goddard in pristine condition, and just jumping once took out half of my right front armor.
I'll go do a few jumps with a faster ship and see if there is any pattern to it.

[evertvorster]

Just played a little more. Every jump now causes damage to the armor. Every jump I did some other part of the armor got damaged, so there is some randomness there. It does not seem to be confined to one ship type either, any ship that you are flying is taking damage through a jump.
Could it be that your current stats are "saved" when you exit a system, and "restored" in the next system, and that it is not completely restored?

[evertvorster]

Tested the PR #901 against this branch, and there is no damage when you jump to a new system now.
From my point of view, this one is ready to go. :)

[evertvorster]

One thing I am noticing with this branch is that in a save game where I am flying around in a Goddard with a SPEC capacitor V, it says I don't have enough jump energy. In master branch the same jump drive with SPEC works flawlessly.

When looking at the SPEC capacitor in the base computer, it says "It was not an upgrade, check the code"

[royfalk]

Can you post the following:
ship info - jump cost and ftl capacitor.
SPEC capacitor V capacity.

[evertvorster]

Hi there.
On the master branch, in ship info it gives the error:
Error: Ship description not found.
And so it does not list a jump cost.

SPEC capacitor V Spec:
FTL Capacitor states 2 500 000 M/J of capacity.

With the task_drive_components branch:
The Ship description is read properly, and explains all the virtues of flying a military spec Goddard.
Jump Cost:
166 666 Mj/s

It does not appear to recognize the SPEC capacitor.

Here is the csv of the milspec Goddard that is displaying this issue:
Goddard.milspec.csv

[royfalk]

This is odd. I looked at the csv and Goddard.milspec is 200 for warp and 200 for jump. It comes with a warp capacitor of 450 so it should work out of the box.

Out of curiosity, can you start a new game with a Goddard.milspec and just fly it. I think the issue is the ftl capacitor overwrote the current number with 0 for some reason. I'll try and get to it today and see if I can figure this out.

[evertvorster]

Hi there.

I started a new campaign, and saved it.
Then I edited the save file to have Goddard.milspec instead of Llama.begin, and when loading that game, in the HUD it displays the message: low jump energy.

The thing with this saved game, though, is that I sold the milspec package some time ago, and installed my own components. I had a bit of spare cash lying around, and could afford more upgraded components than that of the milspec package. Now, if you remove the milspec package, the ship should just be the stock version, right?

[evertvorster]

A bit more information.

Started a new campaign, and went to the shipyards straight away. The SPEC capacitors listed for sale have the same error message as the screenshot I added in to this PR. To me, it looks like the game engine is not properly reading the SPEC capacitor part of Assets. The other components appear to be fine, with descriptions and all.

[evertvorster]

Hi there!

While Play testing this PR, there are a few issues. If these are fixed in another branch, then this one is good to go. However, they do bear mentioning here:

SPEC capacitor details are not available, and Jump drive cannot be purchased.
In the following screenshot, a new campaign was started, and in the upgrade ship area of Atlantis, you can see that the jump drive is red and unpurchase-able. Also, the SPEC capacitor specs are showing an error message.

Saving and loading works, and you have all flight controls in the saved game, so that is a bonus.

SPEC engages, but then a second later disengages. This might be due to the game not reading the proper values for the SPEC capacitor.

Top speed in Llama begin is 120, and after you do your first repair and refuel it is 125, which is what is normal, which means that the game is reading Llama.begin properly.

When loading a saved game with a .milspec ship, you also cannot jump, and on the HUD is states that the jump energy is low.
When loading a saved game with a .stock ship, you can jump, but still in the ship upgrades computer there are no stats on the SPEC capacitor or the jump drive.

Fuel burn rates seem to be normal.

Comparing the same tests against current master, SPEC capacitor has details and engages properly on a new campaign.
You still can't purchase a jump drive, but I do believe that this is fixed in another branch.

In conclusion, it would appear that this PR does introduce the issue where the SPEC capacitor's info is not read properly, and that has some knock-on effects.

[royfalk]

Jump Drive - I know I fixed this in another branch. I'll try and find out where and maybe separate it, if the branch is still in testing.

SPEC - unable to reproduce with Llama.begin. Can you confirm your asset production branch is master and is up to date?

Goddard.milspec - unable to reproduce. Saved a new game and edited to Goddard. Ship goes into SPEC even without the package. Jumps with and without. SPEC capacitor looks fine in upgrade view.

Are you testing the branch itself or the master with the branch merged into it?

[evertvorster]

Hi there!

I am testing just the branch, but I now know how to check out master and merge the branch to it. In my limited understanding of how git works, I thought it would not make a difference if I just checked out the branch, rather than merge it into master.

Will double check that Assets is the latest master, and play test with latest engine with this branch merged into it.

[evertvorster]

This branch has a merge conflict with engine/src/cmd/basecomputer.cpp, and so does not merge cleanly. I don't know how to fix this, unfortunately.

[evertvorster]

Hi there!

Thanks for fixing that merging conflict.
I uploaded a video here to show my building and testing of this pull request as it stands now.
https://youtu.be/xQNEZ0m0sAc
Since we are getting different results, something is different between what it is that we are testing.

The volume on the video is quite low, as there has been no processing done on it, so you will have to turn up the volume a bit to hear properly. Please see if you can see a problem in my testing setup.

[royfalk]

Unfortunately, I discovered I was working on the wrong branch. I intended to work on the drive, which is why it's called 'task_drive_components'. When I actually worked on the drive, I had to name the branch 'task_real_drive_components'.

You can stop play testing for now. Everything reproduces.
So there are several issues here I need to fix:

1. With units now consistent, a ship with with reactor output of 50, warp capacitor of 45 and warp usage of 20 should work. Right now it doesn't. These are the stats of Goddard.milspec.
2. With the move to python, ship view doesn't take the factors from configuration. Need to do so.
3. Need to make sure to reapply factors in the opposite direction when saving the ship. Otherwise, two saves and a factor != 1and you'll see a change in how the game behaves.

[evertvorster]

OK, good to know it was not something on my side.

Let me know when to test this branch again, please.

[royfalk]

Let me know when to test this branch again, please.

Go for it. It should now work even with Llama.begin with reactor02.

[evertvorster]

Hi there!

Just took it for a spin, and SPEC now works in a new campaign. So, technically you can start a new game and be able to just play normally, which I suppose is a good thing for someone new to Vega Strike.

However, there is still something that bothers me.
When selecting a SPEC capacitor in the base computer, it still shows that error message about not being an upgrade.
One other curiosity is that when starting a new campaign, your Llama weighs 250mt, and uses 12000Mj/s for SPEC flight.
My saved Goddard weighs 4898 metric tonnes, and only needs 400Mj/s for sustained SPEC. It feels like something is off there. Mind you, this is only a small detail, and does not prevent you from flying and enjoying the game, it just looks weird.

[royfalk]

When selecting a SPEC capacitor in the base computer, it still shows that error message about not being an upgrade.

I was unable to reproduce that.

One other curiosity is that when starting a new campaign, your Llama weighs 250mt, and uses 12000Mj/s for SPEC flight.
My saved Goddard weighs 4898 metric tonnes, and only needs 400Mj/s for sustained SPEC.

Yeah. I am not a fan of that. At some point in the near future I plan on simplifying assets somewhat. The current thinking is to make warp cost and jump cost functions of mass. That would get rid of these inconsistencies and shorten the file.

I also want to remove support for eight and six facets and make armor four facets. Merge moment into mass. Save four facet shields and front, back, left and right (and not top-left-port whatever).
One problem with this is the fact armor support in the cockpit is for eight. Tricky.

[evertvorster]

I fully agree, stuff that can be calculated should be. It does not really add to the game play apart from a nice description. It's not like a better SPEC makes you faster, or a better jump drive makes a jump faster. What I have noticed is that a bigger jump capacitor allows you to make successive jumps if you are in a particularly fast ship. Which is nice.

That error message with the SPEC capacitor is still present. When you have it selected in the upgrades section, this is the description of the SPEC drive, and the error message is included in the description:

This is with this branch of Engine, and the task_ship_info_3 branch of Assets and starting a new campaign.

[royfalk]

Managed to reproduce it. This should go away when we merge #904.

[evertvorster]

When merging in the other branch mentioned, the issue with the SPEC capacitor description does go away.
With both these branches merged, I cannot find any fault with the game play.
This one is good to go!

[royfalk]

@BenjamenMeyer ?

[BenjamenMeyer]

Please add a log message for the abort