Second footer for Capacity added, shown in equipment market

[PVJeltz]

Issue3117 implementation.
Summary:

• Added two new lines to ship info screen: Cargo space(available for cargo/ship maximum cargo capacity), Cargo space used(same as on the station view).
• Modified TabView to use unique footers for tabs(maybe not the best solution) so on the equipment market screen the capacity gauge is shown instead of the cargo gauge. Drawback: there are two footers now with duplicated ui elements and subscriptions to value changes because a ui element can be attached to one container only.

Leftover/to discuss:

• I needed to add a new text entry: CARGO_SPACE_USED, it is only added to en.json atm.
  Any idea to avoid adding this new entry? or just dump this line?
  I tried with another language and there is no fallback to English(so the game crashed) why is that? Ugly text is better than nothing imo :)

[impaktor]

I tried with another language and there is no fallback to English(so the game crashed) why is that?

Don't worry about it. As soon as this is merged the new string from en.json will be copied over to all other language files before the next build is made. So this crash only happens when you're accessing non-existing language strings, which will not happen once merged.

I'll test this in the comming days, but I can say now that the commits messages make me think a rebase would be in order, since three commits with the same message is undescriptive I think.

Also, I don't know if your editor is changing indentation, so please make sure you adhere to the indentation used in the file you're editing.

[PVJeltz]

Yes the last two commits are me fighting with the tabs and eclipse :)
I'm new to git and I was lazy to change the comments, but I will next next time.(Didn't expect you will see every commit)

[impaktor]

If you want to fix your commit history, you can read a short guide here and have a go at it, if not, we'll fix it when merging.

[impaktor]

There is a trailing tab here.

Actually, there are trailing spaces and tabs on quite a few places (6 in this file). I see them using git diff master from your branch. There's also some places where indentation is done by spaces and tabs, instead of just tabs.

I have my editor set up to nuke all trailing spaces, tabs, and lines when I save my files. I assume you can do something similar in eclipse.

I also assume eclipse can indent the code using either spaces or tabs.

Sorry for nitpicking. But a git commit --amend is your friend when this is fixed.

[PVJeltz]

Thanks, bad bad eclipse.... (I configured it to show trailing spaces use tabs etc...).
I can fix it after work.

[impaktor]

I find an image is useful, to those too lazy to test this branch:

Normal footer when not in Equipment market:

...but now in Equipment market, it shows Capacity instead:

[PVJeltz]

I hope it is fixed now:)

[impaktor]

I hope it is fixed now:)

Looks good.

Just one more complaint from me: commit message. I think something like "Second footer Capacity added, fixes #3117" would be more descriptive, and would also have the pleasant side effect of closing the bug report when the commit is merged. (a git commit --amend fixes this).

Other than that, I'd like some of the more lua savvy persons to give a 👍 on the code, like @laarmen before final merge.

After this much complaining from me I'd just like to say that this is great work, and I like it, and thanks for contributing.

[PVJeltz]

No problem, thanks for the review.

[laarmen]

It is on my to-do list, I hope I'll be able to review and merge this
afternoon.

[laarmen]

Bear with me, this is a cellphone comment.

In your function makeFooter (or is it add footer?), you pass a string to differentiate the gauge construction. Since otherwise your code seems to take into account other specialised footers, could you please instead pass directly the gauge, and leave to the caller its construction? That way, when impaktor will come up with another crazy idea needing a fuel gauge or whatever, he won't have to modify the function, only to call it with the appropriate gauge.

Also, for your future contributions, could you split the commits that modify an API from those making use of it? This makes the review much easier, amongst other things.

[PVJeltz]

I can do it on the weekend only, so if you want to get rid of this pull request earlier please fix it yourself(btw can you do it or I should give you the permission?).

[laarmen]

I'll just implement it in a separate commit and leave it for others to review and merge it, I'll merge this one as is.

What saddens me a bit, is that the root of the problem isn't solved. However, as it is a design decision on my end that caused all the fuss, I can't expect you to solve it for your first contribution, now, can I? All in all, this fix is still much better than what we have.

(you can skip to the end if you wish, this is more for documentation)

As a (not so) small reminder to us all, the problem actually is that before #1719, the cargo bay capacity was conflated with the overall capacity of the ship, despite having two distinct configuration keys in the ship definitions. Thus, the engine pretty much has this conflation assumption built-in, and I might have missed other places where the code is semantically wrong but used to work thanks to that.

As is turns out, there was ONE ship that didn't have the same value for capacity and cargo size, I suspect as a result of a typo. As the pre-n-e code pretty much only used capacity, it didn't matter, but some parts of n-e fixed these bad semantics and thus resulted in unexpected behaviour for seasoned Pioneer players.

(end of the documentation bit)

Anyway, thanks for the fish! Under what name (or pseudonyme) do you wish to be listed in the AUTHORS file ?

[PVJeltz]

Add me as PVJeltz.