Sitrep panel icon / text size adjustment

[The Silent One]

As mentioned in this thread viewtopic.php?f=10&p=78809#p78809, I am trying to make both the icon and the text size of the sitrep items editable; and kinda succeded . But with this, another problem came up: if the icon is larger than the text, it will get cut off by the panel border (see below). I would fix this by checking if the text or the icon is larger, and using the larger figure to render the panel border.
However, I have several problems:

- not sure where exactly in the code the panel border is rendered. is it here?
class SitrepDataPanel: [...]

Code: Select all

virtual void        SizeMove(const GG::Pt& ul, const GG::Pt& lr) {
            Init();
            const GG::Pt old_size = Size();
            GG::Control::SizeMove(ul, lr);
            if (old_size != Size()) {
                DoLayout();
            }
            if (m_link_text) {
                // Use the height of the text as our height.
                // DoLayout reflowed the text.
                GG::Pt text_size = m_link_text->TextLowerRight() - m_link_text->TextUpperLeft();
                text_size.y += ITEM_VERTICAL_PADDING*2; // Text centers, so this puts padding on both above and below
                text_size.x = lr.x - ul.x; // Ignore the width of the text, use whatever was requested.
                GG::Control::SizeMove(ul, ul + text_size );
                DoLayout();
            }
        }

- I intend to do something like

Code: Select all

if (GetIconSize() > text_size.y) {
    text_size.y = GetIconSize() }

... which isn't working because GetIconSize() returns the type GG:X while GetIconSize returns a GG:Y... while both are essentially integers. (How) is it possible to do a typecast?

[Dilvish]

I'm not understanding what problem you are trying to solve here (overall, not about the situation with the icon being too much larger than the text)-- I don't get what you mean by "At large icon sizes, the text will get too large in relation to the icon."

As for the comparison, I kind of had it in my head that GG had some more direct way to manage something like this, but I'm not finding it now (maybe I'm just thinking of something from Font). X and Y are essentially just renamed ints-- if it won't let you directly cast from Y to X, then perhaps you can try

Code: Select all

GG::X(int(some_ggY_value))

[MatGB]

I'm not understanding what problem you are trying to solve here (overall, not about the situation with the icon being too much larger than the text)-- I don't get what you mean by "At large icon sizes, the text will get too large in relation to the icon."

Go into config.xml and change the icon size to 32 (ie twice the current size). Do nothing else.

Boot the game up and look at the sitrep panel

Say hello to my little friends

[The Silent One]

Go into config.xml and change the icon size to 32 (ie twice the current size). Do nothing else.

Boot the game up and look at the sitrep panel

... see here:

[MatGB]

Exactly, I'd love to have the icons that size, but I can't stand havign the text that size.

[The Silent One]

As for the comparison, I kind of had it in my head that GG had some more direct way to manage something like this, but I'm not finding it now (maybe I'm just thinking of something from Font). X and Y are essentially just renamed ints-- if it won't let you directly cast from Y to X, then perhaps you can try

Code: Select all

GG::X(int(some_ggY_value))

Figured it out:

Code: Select all

if (Value(GetIconSize()) > Value(text_size.y)) {
					text_size.y = GG::Y(Value(GetIconSize()));
				}

Result (icons 32/ text 14) below. Ok to commit?

[Dilvish]

Ok to commit?

The feature itself certainly looks fine, if that's what you're asking. As for the code changes, it's not clear to me if you are saying that the 3 lines quoted above were all it took, but even if that's the case, since you don't have any recent history making c++ code changes in FO you should probably just start out submitting PRs so that Geoff might comment. For example, even just for the 3 lines you quoted above, I expect that Geoff might comment that the braces are unnecessary (so it can be just 2 lines), and that the second line is (or at least in this representation appears) over-indented (indentation and similar formatting issues are something we try to be consistent with).

[The Silent One]

The feature itself certainly looks fine, if that's what you're asking. As for the code changes, it's not clear to me if you are saying that the 3 lines quoted above were all it took, but even if that's the case, since you don't have any recent history making c++ code changes in FO you should probably just start out submitting PRs so that Geoff might comment. For example, even just for the 3 lines you quoted above, I expect that Geoff might comment that the braces are unnecessary (so it can be just 2 lines), and that the second line is (or at least in this representation appears) over-indented (indentation and similar formatting issues are something we try to be consistent with).

Sure, I can do a PR. It took a little more code changes than above, and I would find it helpful to have someone experienced look over my code.

[Geoff the Medio]

...the second line is (or at least in this representation appears) over-indented...

I'd assume that's due to not-careful use of copy+paste into code tags.

In addition to Dilvish's points, I think changing text_size is misleading. Instead it should use the max of text_size and icon_size when making the call to SizeMove. Possibly an additional temp variable would be used to implement this.

[The Silent One]

Is this what you had in mind:

Code: Select all

GG::Pt panel_size = m_link_text->TextLowerRight() - m_link_text->TextUpperLeft();

// use icon or text size, whichever is larger
int maxPanelSize = Value(GetIconSize());
if (maxPanelSize < Value(panel_size.y))
maxPanelSize = Value(panel_size.y);

maxPanelSize += Value(ITEM_VERTICAL_PADDING);
panel_size.y = GG::Y(maxPanelSize); // Text centers, so this puts padding on both above and below
panel_size.x = lr.x - ul.x; // Ignore the width of the text, use whatever was requested.
GG::Control::SizeMove(ul, ul + panel_size );
DoLayout();

[Dilvish]

I'd assume that's due to not-careful use of copy+paste into code tags.

No point in assuming when one can simply explicitly note the uncertainty.

[The Silent One]

Edited post above.

[vincele]

Or that kind of things:

Code: Select all

int maxPanelSize = std::max(Value(GetIconSize()), Value(panel_size.y));

[The Silent One]

Ooooh me likes! Thanks. I've changed the relevant lines.

[The Silent One]

I've done a little code grooming, compiles and works as intended.
Pull request is on GitHub:
https://github.com/freeorion/freeorion/pull/181