Avoid drawing vehicle equipment at (0, 0)

[dl471]

Proposed fix for issue #796. Since the source of the problem appears to be the location of the inventory on screen being off for the first few frames avoiding drawing the inventory until the correct coordinates come along should fix it.

[JonnyH]

It feels like skipping any drawing to {0,0} is a hacky workaround - I think the best solution would be to find why the positions aren't yet set, and ensure that they're correctly positioned before the first draw call.

I suspect the Forms don't setup their initial location correctly, perhaps calling the recalculate-position functions whenever a form is modified (e.g. a new child added) would fix that?

I can see this biting us in the ass later, as it may be possible for a modded UI to have {0,0} as the expected position, then we get bug reports on why it's not drawing correctly, or the 'wrong' value stops being {0,0}, and we get the same incorrect position issues.

[dl471]

That's a good point. In retrospect I was making quite a lot of assumptions about presumed correct behaviour.

I'll dig around the Forms and see if I can get a better idea of what's going on with it.

[dl471]

Sorry for disappearing for a while.

As best as I can tell the issue arises when forms are copied. The initial location is correct but when the form is copied the resolvedLocation is not copied with it. This leaves it at zero until the next time update() is called. This can be resolved by copying the resolvedLocation when making a copy or having setParent call update() when it is called but these seem to reintroduce issue #572. That in turn can be resolved by having ListBox and MultiListBox update themselves when new items are added. I've pushed commits that show what this would look like.

Alternatively, both of those issues can be solved by calling the update function for the vequipscreen and vehicleselectbox at the end of the vequipscreen's begin() function. Not as general a solution as above but it has the advantage of localizing any changes made to that function alone. Since my first attempt changed various files and caused weird behaviour that could crop up elsewhere trying to limit the scope of the change like that might be preferable.

Would you have any comments on these approaches?

[emptyVoid]

Looks neat to me.

Though for the sake of optimization I would prefer the second approach -- update lists after populating them with all the items.

IMO better yet would be to use the first approach (updating on item insertion) with an ability to suspend updates, e.g.:

class Control : public std::enable_shared_from_this<Control>
{
  private:
    int updateSuspensions = 0;
    bool pendingUpdate = false;

  protected:
    bool isUpdateSuspended() const { return updateSuspensions > 0; }

  public:
    void Control::suspendUpdate() { ++updateSuspensions; }
    void Control::resumeUpdate()
    {
      --updateSuspensions;
      if (updateSuspensions == 0 && pendingUpdate)
        update();
    }

    virtual void update()
    {
      if (isUpdateSuspended())
      {
        pendingUpdate = true;
        return;
      }

      pendingUpdate = false;

      ...
    }
}

So that forms with lists (such as VEquipScreen) could suspend updates while populating.

Although this would require adjusting logic of all the controls.