search

phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

clock is not centered in game status bar #80

Closed pixelzoom closed 3 years ago

pixelzoom commented 4 years ago

In some sims, the clock is not centered in the game status bar (FiniteStatusBar).

Noted while working on https://github.com/phetsims/tasks/issues/1044, conversion of ElapsedTimeNode to ES6. I thought this might have been something I did in the conversion, so stashed my changes. It's unfortunately a problem and impacts all sims that have a game.

It looks fine in BCE, RPAL, and the vegas demo.

But in these sims, the clock is quite a bit below center:

Balancing Act:

screenshot_549

Build An Atom:

screenshot_551

Graphing Lines:

screenshot_550

Assigning to @ariel-phet to prioritize and assign.

pixelzoom commented 4 years ago

The clock and the 3 nodes to the left of it are added to an HBox. It looks like the alignment it 'top' instead of 'center'. But the code is definitely 'center'.

ariel-phet commented 3 years ago

@pixelzoom normally I would defer this until one of these sims was getting worked on again and just have sim specific issues made. However, Balancing Act is in the iO pipeline in the near'ish future so it is worth at least investigating if this is reasonably easy to tackle.

Please put in 2 or so hours of investigation after the 10/1 Natural Selection deliverable. And assign back to me after that investigation (if fixed in that time great, if not, we will hand off the baton)

pixelzoom commented 3 years ago

Fixed in the above commits. This took me 15 minutes.

This bug was introduced when the excludeInvisibleChildrenFromBounds option was added to LayoutBox for https://github.com/phetsims/joist/issues/608. The default is true. We need to set it to false here, because if a game keeps track of time, then the clock needs to be considered for layout regardless of whether it's visible.

Back to @ariel-phet to decide if this needs to be reviewed by another developer. Low risk -- I added one line of code, and documented why I added it:

    // Because elapsedTimeNode needs to be considered regardless of whether it's visible,
    // see https://github.com/phetsims/vegas/issues/80
    excludeInvisibleChildrenFromBounds: false,
ariel-phet commented 3 years ago

@pixelzoom no need for a review.