colorful notifications

[alexban01]

Videos are out-of-date

https://user-images.githubusercontent.com/21990802/176780246-647a40f8-6c82-459b-98da-d099ef965871.mp4

[SomeTroglodyte]

You're instantiating a Color object repeatedly, and Color is a kind of object used by Gdx that can be reused in several places (unlike e.g. an Image Actor). You could do enum class NotificationColor(val color: Color) { Default(Color.WHITE), Construction(Color(0.745f, 1f, 0.698f, 1f)),... and these Colors would be singletons, and you could even see to it the enum names get serialized instead of the Color values, which may be much more compact (perhaps, I'd actually look at the json to see how verbose it gets). Just food for thought, choose what you like best.

[alexban01]

enum class NotificationColor

that would actually be much cleaner. I'll convert this to draft and will do that tomorrow. :)

[AdityaMH]

And Color darkens after selecting notification

[yairm210]

I think we should Discord-poll whether this is better or worse, I'm not sure I like this visually. Also, if we do this, then 'has been built' notifications should be light brown, as per Production color

[alexban01]

I'm not sure I like this visually.

I'm pretty sure I can make these colors toggleable in settings for people who don't want them

It helps people tell more easily what happened in a turn without having to read every single notification as most of them aren't that important (city has expanded its border, or city has grown!)

[AdityaMH]

"x was destroyed when attacked our unit", or "x being razed to the ground" set it as bad or good?

I think military related should be different color.

[alexban01]

I think military related should be different color.

Well if its good its green, if its bad its red. The notification scroll should have as few colors as possible to not turn it into a rainbow mess

[alexban01]

Discord-poll

@yairm210 could you do it? I can't seem to add pictures or videos in that channel and this poll probably requires showing something like this, although this doesn't show the VeryBad notification (the one in red). If you can maybe put the video I made and posted in the PR message instead of the picture.

[AdityaMH]

bombard color should red because part of enemy come

[Azzurite]

First of all:

as few colors as possible to not turn it into a rainbow mess

The LGBT rainbow flag has 7 colors. There are 8 different colors currently defined in NotificationColor, not including the default white. Isn't it already a rainbow mess?

---

Secondly, I've seen the poll and it's overwhelmingly in favor, but I don't think that the poll is really relevant because there's no indication of which things are exactly color coded how. In general, I completely agree that color-coding the notifications might make sense. However, how exactly should different things be color-coded? That's not mentioned in the poll, so I question a bit how useful the poll really is :D

---

Thirdly, what is the purpose of color-coding? I feel like there are two possible purposes: To highlight something important that may otherwise be lost if it weren't color-coded i.e. to bring something important to attention or to make something visually distinct to be able to group it better.

So which of the two are you actually doing here? I would argue that you're doing neither. Is it really important that the seaport in Ghost Mountain was built? Do you really need a special distinction for this game-changing event? I'd say no.
Wouldn't it be more important to be highlighted what started building, which is currently white? You'd maybe want to know what the computer assigned as production automatically for you, and change it if you don't like it.
"M'banza Kongo has given us 15 gold for meeting us" is important? Definitely not.

So what about the second purpose? Does it group events together nicely? I would also argue that no, not in its current state. Does it group all enemy unit actions? No, that an enemy was spotted in our territory is still white, or that our fishing boats were pillaged is also white.

So I like the general thought and approach, but I absolutely do not like the actual implementation of the approach.

I think there are some very good uses here for the color-coding, but let's first look at how the current notifications look like. Currently, the notifications already have an icon on the right side, which is basically like color coding, just not quite as visually distinctive. If you look at the string of construction notifications, they all have that nice brown gear icon. So the second purpose of color coding is already fulfilled there. The second purpose of grouping the notifications together is also fulfilled because similar notifications all occur one after the other, i.e. the construction notifications are not interspersed with combat notifications.

So IMO, the only thing we need to be looking at for color-coding is the first purpose, highlighting importance. There certainly are a couple notifications that are more important than others, but as I said, they aren't really highlighted in the current implementation. In general, I think you know which notification is important by thinking about "Do I need to take action because I now know about this?"

For example, I think the notification for one of our improvements being pillaged is an important notification to know about, because we will need to take action because of it. An improvement was pillaged, so we need to get rid of the enemy unit and send a worker over to fix the improvement.

Another example of an important notification is that a trade has ended. When a trade ends, you'll likely want to take action in that you want to re-do the trade, if you're still friendly of course, but at least you want to think about it.

An example for a notification that is not important is that some building has been built: there are maybe a couple exceptions, but generally you don't care about that notification because you'll get asked to either choose a new production by the "next turn" button (so the notification is unnecessary), or, if you have auto-build on, then the important notification is actually the one about which new thing is now building, because you might want to take action if that is not something you'd like to be built, and choose something else.

---

There's also just a bunch of general mis-classifications:

Look at this part of the code, Battle.kt line 338: defender.getCivInfo().addNotification(notificationString, ... , color = NotificationColor.VeryBad). So whatever is happening there is apparantly "Very Bad". But which notifications really go in there? What can notificationString be? If you look at it, notificationString can for example be [enemy unit] was destroyed while attacking [our unit]. Isn't that good? Or it can also be [enemy unit] has attacked [our unit]. That's not good, but also not "very" bad? Maybe just bad? Or maybe it was good because the enemy unit took much more damage than ours, so it's exactly what we wanted to happen?

---

The last thing I think is that it's important for the user to know which color actually means what, currently, there's absolutely no indication what the purpose of each color is, the only real way to find out is by reading the code, or at least the names of the enum vars.

---

So to summarize, I think it's a really good idea to color-code the notifications, and I don't really mind the visuals - except for there already being 8 different colors, which already is too rainbow like you mentioned. It should at most be 3 or 4. And then, it really needs to be looked at what exactly needs to be highlighted in which way, because I think the current color-coding is pretty useless unfortunately. Currently it's more like how some students highlight entire passages in their textbook, and when everything is highlighted, the highlight is essentially useless.

I haven't thought about this very long, but I would rather color-code it kind of like this: "default/white" - just some random info, doesn't need any action, "good to know" - something that's important to keep in mind, but probably does not require immediate action, "needs action" - something that requires attention during this turn or at least very soon, and "requires immediate attention" - some game-deciding things, like that one of your city has been captured or that someone has built the United Nations or stuff like that, which really changes the direction of the current game.

[OptimizedForDensity]

Maybe a happy medium could be creating the colored notifications framework then letting users select what color certain notifications are? Maybe you want to see at a glance which notifications are brown (which you, the player, designated as the "Construction" color and use it for construction started/aborted/finished notifications). This would probably involve an enum for each type of notification then an options popup with a color picker (16 colors preset colors?) that lets the user pair the notification type enum with a color. It could also work well with a separate feature where users can disable certain notifications altogether if they want (just an idea to reuse the same options page/popup).

This is more of a general idea and something that could be implemented over another PR or two. I like the colors and support them in either their current form or as a subset of the ones you've current programmed.

[AdityaMH]

We just need red, brown, crème only. no rainbow. Red for military thing, brown for building, crème for trade, gold stuff, wltk. and other is white.

[AdityaMH]

Or add switch setting to set off (turn off colorful notification), simple (just red, brown, crème, or white), detailed (rainbow) *not recommend yet.

[alexban01]

So here's how I colored these: I'm still very open to suggestions, especially since I don't really have many VeryGood ones

VeryGood:
    Our ___ intercepted and destroyed ___
    Your [citadel] has destroyed an enemy ___

Good:
    CS can gift great people
    CS assigned you a new quest
    Accepted trade request (not sure if this is important really)
    New ally
    ____ and ___ have signed Peace Treaty
    We have looted ___ from a ___
    Agreed to stop settling near us
    ____ and ___ have signed delcaration of Friendship (other civs also get the "Good" color)
    One time free:
        unit(s),
        unitRuins,
        policy(s),
        greatPerson,
        tech(s)+techRuins,
        revealEntireMap
    One time gain:
        prophet
        pantheon
        trigger voting

VeryBad:
    Our ___ was destroyed by an intercepting __
    Enemy was spotted in/near our territory
    after being hit by our nuke, ___ declared war on us
    A(n) ____ exploded in out territory

Bad:
    Broken pledge to protect
    CS canceled quest because you demanded tribute
    Offered counter offer (it's bad because he didn't acccept what you wanted?)
    Proposed trade no longer relevant
    Trade offer has ended
    _Protector_ is upset that you attacked/demanded...
    _Protector_ is outraged that you destroyed && attacker destroyed ___ you you pledged to protect
    Lost ally
    Cannot provide upkeem-> unit has been disbanded
    Declared war on us (the rest of the civs still get a white notification)
    Denounced us (same as above)
    Relationship with ___ is about to degrade/has degraded
    Refused to stop settling near us
    One of our trades with ___ has been cut short (not sure if this one needs a color though)
    ____ denied trade request
    Barbarians raided and stole gold
    Enemy captured our __
    Your ___ plundered __ from __
    Your citizen are revolting
    Our __ took tile damage
    Enemy has pillaged our ..
    Your territory has been stolen by ___

Construction:
    City changed production
    Unit became obsolete and was removed from queue
    Work started on ___

Plus:
    defendingUnit withdrew from a attackingUnit: Good for defender, Bad for attacker

[alexban01]

At the moment the notification scroll doesn't updated after changing the setting. Not sure why updating the worldScreen doesn't update the scroll as well

Oh yes and I still haven't fixed the Battle.kt issue yet. Gonna have to draft this again actually

[Azzurite]

I think there should be a setting to turn this off if someone doesn't like it, what do you think? Seems to be a relatively major change and pretty easy to add an option for. I for example would probably turn this off if possible :D

And I still think it's pretty hard for people that are not reading the code to understand what the colors actually mean.

[alexban01]

And I still think it's pretty hard for people that are not reading the code to understand what the colors actually mean.

Is it? I mean there is red for bad stuff, green for good stuff and white for meh stuff :))

I like OptimizedForDenisty's idea with customizable colors and with a couple presets it probably shouldn't be hard to do. Or maybe I should just make these moddable?

[github-actions[bot]]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

[yairm210]

Superceded by 7edfd497

[alexban01]

ahh yes thanks.. I tried doing what Azzurite suggested but it turned out to be out of my league since one of the notifications (can't remember which) could be both bad and good depending on the situation.. and defining a color for that was impossible

BUT your implementation is much cleaner.. I didn't thought about simply dividing the notifications like that

[alexban01]

but the text is a bit small on the "Huge" screen size

[AdityaMH]

Rather than this, big round shape with icon and color. and text only appear when press hold to screen (android) or, icon hovered from mouse pointer. more better. But since new sortable notification added, color thing will be useless.