FAForever / fa

Lua code for FAF
221 stars 228 forks source link

ConstructionRework #6148

Open clyfordv opened 2 months ago

clyfordv commented 2 months ago

Start of construction.lua and layout rework.

Main addition is a new ConstructionPanel class (in constructionpanel.lua) that handles all the instantiation and layouts for the background elements of the construction panel. Additionally, the ConstructionPanel has logic to switch between displaying the tech tab background (behind the radio buttons that allow you to switch tech levels/go to the template selector), or the normal background that displays when you select a non-construction unit. Welcome any input here, it's still in the exploratory phase.

To see the two different layouts (tech tab and normal), just select your ACU multiple times, there's a flag set up to cycle through them. Tech tab background is (intentionally) ridiculously long.

All changes outside the ConstructionPanel file can be described as provisional (read: janky), but include:

Garanas commented 2 months ago

I think this is taking a good direction but I'm also afraid that it may spiral out of control. It's always better to have many, relative small pull requests that tackle the problem in stages (one stage per pull request) then one large pull request.

@clyfordv try to see if you can come to a conclusion in this pull request. A conclusion being that the current work doesn't break any functionality, but it does progress your goal (reworking the construction panel). Then we can merge this stage in, and pick it up again in new pull request.

See also:

And a personal example that, uuhh - a bit too much in one pull request:

clyfordv commented 2 months ago

I think the "spiral out of control" ship may be sailing (tbh I wasn't sure this PR specifically was a target for merge, just wanted people to be able to see what I was talking about on the discord).

I think we can break it down into: PR 1: Introduce new components, replace constructionmini.layout, leave construction.lua mostly unchanged. PR 2: Re-implement the left and right layouts to put the above framework through its paces + patch up the (currently woknky, even in deploy/fafdevelop) Layout switch system. PR(s) 3 -> N: Clean up the construction.lua logic, which has two primary functions (processing the incoming selection data--a big one, with lots of bug fixes bolted on over the years--and giving orders to units).

Garanas commented 2 months ago

Breaking it up sounds fine too. My point is that there's so much time spent on this at the moment that it would be a shame if we'd lose it all because of a feature creep.

PaletzTheWise commented 5 days ago

Hi @clyford, If I understand it correctly, the current construction.lua is the entire bottom construction-selection panel. You seem to have identified a better way to approach UI and also functionality, in particular UI elements, that may be extracted into separate classes/files.

The stages you suggest don't seem to narrow their scope much, I suggest that you should instead extract one class/file at a time, starting with the simplest first.

I don't follow exactly how you intend to dissect construction. What is the difference between construction.lua and constructionpanel.lua? Is the latter just a better replacement of the former, perhaps with all the classes already extracted? If so then there won't be construction.lua in the final state, right? What individual UI elements, abstract or actual, are you planning to extract?