Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New Feature/Toolset: Paradox Bar! #104

Open
Azurency opened this issue Sep 23, 2017 · 23 comments
Open

New Feature/Toolset: Paradox Bar! #104

Azurency opened this issue Sep 23, 2017 · 23 comments

Comments

@Azurency
Copy link
Owner

Issue by chaorace
Sunday Apr 30, 2017 at 00:40 GMT
Originally opened as CQUI-Org/cqui#514


What is this?

Okay, so there's the name... It's basically an imitation of the the notification scheme found in the grand-strat games published by Paradox, like Crusader Kings, Europa Universalis, and Stellaris. It looks like this if you're drawing a blank.

Right, so that's the idea, implementing a saner notification scheme that's easier to read. How far along is this pie-in-the-sky dream? It's complete! So, why is this a PR instead of just going direct-to-game? Well... there's only two notifications implemented so far, and they're both purely for debugging purposes. That being said, the internals have been tested extensively and the "API" of this element has been engineered specifically for robustness, flexibility, and ease of use. It's ready for people to start throwing things at it now!

Seeing as we've run short on low-hanging fruit recently, which has dried out the flow of commits somewhat, I've decided to put this PR up as an additional avenue for both the @CQUI-Org/feature-team and outside contributors to quickly and easily extend CQUI.

So, what's the plan?

The plan is to implement notifications that do not exist currently into this new notification system. Once we're confident with the number of notifications implemented, we'll soft-launch this while keeping the old notification system. This way we'll have a chance to gather some user feedback on performance, bugs, and usability without really risking bricking up the notifications that already exist in the base game. After that, we'll see about moving over all notifications to this new notification platform, along with implementing much-requested features like snoozing notifications and muting specific notification groups

Why abandon the existing notification bar?

It's difficult to mod, plain and simple! The difficulty present in extending the existing notification system makes it a very unpleasant proposition and is the reason we've been unable to tackle the many existing requests for adding new notifications. It's at the point where we had to instead hijack the gossip popups instead, even when they weren't necessarily the best choice. Moving over to our own custom solution also comes with the added benefit of being able to add notification stacking, a much requested feature.

Beta

It's been too long, I've been putting off the beta tag since November. Let's finish up this last feature and finally release the first beta build 😎


chaorace included the following code: https://github.com/CQUI-Org/cqui/pull/514/commits

@Azurency
Copy link
Owner Author

Comment by chaorace
Monday May 01, 2017 at 01:19 GMT


Heads up, I'm working on improving the LOC text support for notifications, so please hold off on anything related to this PR for a few hours while I polish this up, changes will be breaking

@Azurency
Copy link
Owner Author

Comment by chaorace
Monday May 01, 2017 at 02:57 GMT


Okay, I've overhauled the AddNotification method somewhat, this should be the last truly breaking change, since the new way of providing properties is ordering agnostic

@Azurency
Copy link
Owner Author

Comment by chaorace
Monday May 01, 2017 at 02:58 GMT


Also added better LOC support so it should be considerably easier to create a template string now while filling in the blanks at time of instantiation with a property override upon the new ttprop table

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Monday May 01, 2017 at 12:25 GMT


Got some time to try it out in civ today, and have some comments.

Lets say you create 3 notifications in the same group and remove one and then add new ones the position gets messed upp:
image

The animations look a bit yanky, especially when removing a notification. Right now it goes over the top bar might look better if it started to fade out as well or if it went under the top bar.

It would be nice to be able to remove an entire group with one click, so maybe add an X button or something like that.

When all buttons are added to the worldtracker the notifications overlap the hide worldtracker checkbox.
image

Also I'm wondering if not AddNotfication might need another parameter or so. Say we want to create a notification for city growth, this notification should on left click take you to the city in question. To do this we need to have a city id or something like it. Otherwise we would need to pass a function for in the func parameter and then we can't have a default which we want to still have right click to remove and so on.
Another thing I just thought about, we need to make sure that a notification is only shown for the player it concerns, this is very important for MP but also for SP since I don't want notifications about AI stuff.

Otherwise I think the look is spot on! Great work.

@Azurency
Copy link
Owner Author

Comment by chaorace
Tuesday May 02, 2017 at 02:28 GMT


Yeah, in terms of the animation I wasn't entirely happy with it, but I was hoping it was just because I was being critical, now that I have some feedback I'll look into smoothing it out a bit. I'll probably just swap it out for a alpha transition instead, because I think that will bypass the second visual issue you describe. Speaking of appearance, do you guys think it looks better without the background banners? I'm very back-and-forth on the subject

As for the parameter suggestion, the params table effectively allows passing whatever arbitrary data you want. The params table is accessible from any given func as the third parameter being passed to it, so it should be a relatively simple proposition with the tools already available.

I'll see about adding a close button to the group instance

I don't think it's possible to design a foolproof safety against notifications showing for the wrong player. Really, it's up to whomever is programming a specific notification to filter events so that only relevant ones eventually create a notification.

@Azurency
Copy link
Owner Author

Comment by chaorace
Tuesday May 02, 2017 at 03:04 GMT


Ah, the weird animation behavior has to do with some instancemanager behavior I did not expect!

As it turns out, releasing an asset from a given IM just hides and disables the element. The created instance lives on in memory and will return next time a new instance is needed. This leads to some funny side-effects, like the janky positioning you noticed, if you aren't careful to restore any mutable states to their defaults at the time of summoning

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Tuesday May 02, 2017 at 08:22 GMT


Overlooked the prop parameter, this will do what I asked very well.

I like the background banner, it feels like it matches up nicely with the civ icons on the right.

Edit:
Just thought about something else: icons from atlases. A lot of the vanilla icons are stored in atlases and as such x and y offsets are needed in SetTexture eg:

local textureOffsetX, textureOffsetY, textureSheet = IconManager:FindIconAtlas(iconName,38);
if (textureOffsetX ~= nil) then
 Controls.ReportsImage:SetTexture( textureOffsetX, textureOffsetY, textureSheet );
end

@Azurency
Copy link
Owner Author

Comment by JHCD
Tuesday May 02, 2017 at 09:06 GMT


Do I have to activate somthing? Nothing happens here...

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Tuesday May 02, 2017 at 11:02 GMT


@JHCD You have to use Firetuner to test it, atm it is not hooked up to any events and only have debug notifications.

@Azurency
Copy link
Owner Author

Comment by JHCD
Tuesday May 02, 2017 at 11:05 GMT


Okay, no idea how that works... Use Firetuner only for logging...

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Tuesday May 02, 2017 at 11:10 GMT


I can probably fix some informative screen shots later today.

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Wednesday May 03, 2017 at 16:36 GMT


First you want to select the correct state, in this case cqui_toplayer:
image

Now you have access to all functions declared in that state. So we can write AddNotification("debug") in the input textbar:
image
Press enter and you should see the notification.

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Wednesday May 03, 2017 at 17:02 GMT


Did some testing with icons and have some more feedback:

  • The last notifcation should have as much margin bottom as the first one have at the top
    image
  • To get the icon in the screen shot above I had to change SetTexture to SetIcon, but when I did that Controls_Circle did not work. I think we might need a more complex icon handling system for this since Icons/images are handled very differently all over vanilla and these icons are nice to use.
  • There seems to be some weird line at the bottom now, it was not there before.
    image

@Azurency
Copy link
Owner Author

Comment by chaorace
Thursday May 04, 2017 at 23:49 GMT


I'll see about possibly autodetecting the image/icon difference. Worst case scenario: I think we could get away with just attempting both functions blindly and swallowing any exceptions. The background banner needs to be at least a certain width, otherwise the transparent textures on the bottom overlap and look ugly like that. I'll look into widening those banners slightly, as well as nudging the icons a bit up relative to the banner

@Azurency
Copy link
Owner Author

Comment by chaorace
Thursday May 11, 2017 at 04:48 GMT


Okay, I've further tweaked the AddNotification method and also changed how new stock notification types are added so that they can be extended if a similar type already exists. These are breaking changes, so sorry about that!

Aside from that, you can see two new notifications have been implemented! Now you'll be alerted whenever a city grows or shrinks. Unfortunately, the game isn't very descriptive about what happens when a city population changes, so this relies upon a nasty heuristic. Keep an eye out for this notification since my personal testing can only do so much!

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Thursday May 11, 2017 at 18:52 GMT


I'm traveling this weekend and we are painting in the computer room so won't be able to test for a while.

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Thursday Jul 06, 2017 at 12:14 GMT


Started a new game today with this on and have found some bugs/problems:

  1. When I first settled my city the bar notified me that the city population had shrunk. If possible it might be best to just skip a notification for this scenario since I know I just founded a city.
    bar_city_founded_bug
  2. I found a natural wonder with the bar still up and it does not hide like the rest of the UI:
    bar_shown_wonder_bug
  3. When I built a settler I got a notification that the population had increased instead of decreased.
  4. The bar is visible when dialog with other civ is open:
    image
  5. When conquering a city I get two notifications, on for shrinking and one for growth
  6. The bar still covers part of the worldtracker buttons:
    image

@Azurency
Copy link
Owner Author

Comment by chaorace
Saturday Jul 08, 2017 at 16:29 GMT


Thanks for testing!

The initial notifications are indeed a bug, just one I haven't gotten around to addressing since it's helpful for testing purposes.

The second issue is because toplayer doesn't respect BulkHide yet, but because the worldtracker keeps bugging us, I think I'll just move the notifications to the same "layer" as the worldtracker, bypassing the positioning and hiding issues entirely

Third and fifth issues are caused by the unfortunate fact that there's only a populationchanged event (we have to infer if this is a growth or shrink event by the city growth, which isn't always accurate, like here), the best solution to this specific issue is probably ignoring growth/shrink events that happen during your turn, since we're mostly concerned with notifying about growth/shrink events the player doesn't directly cause.

@Azurency
Copy link
Owner Author

Comment by SpaceOgre
Saturday Jul 08, 2017 at 18:20 GMT


True, I wish the API:s where better :(

I guess we need to store that information our self if we want a better check. We could store current population for each city on end turn, then we can check against that. Or something like it, but it would take a bit of work ofc.

One more thing I wonder how it would work when a city gets nuked (not that the AI have nuked me yet but I guess it could happen).

@Azurency
Copy link
Owner Author

Azurency commented Oct 1, 2017

I'm not sure what to do with this, I like the idea of having a more flexible notification system but I would greatly prefer to find a way to integrate this in the base game notification panel and not create a whole new paradox games like panel.
I tried today to add custom notifications on the default panel and managed to get something working so I'm sure it's definitely possible.

@winneon
Copy link

winneon commented Jan 9, 2018

Any updates on this? The base notification system is pretty terrible and many people, including myself, would be ecstatic to have this kind of inclusion.

@Azurency
Copy link
Owner Author

I was trying to test another system, stacking notification in another way that what the default game does but in the same place not in Paradox game's way. I was in the "messing around" part but then the fall update came and I did not work on it afterwards.

This Paradox bar solution however is working and already advanced but we'll need more review on it, for me it feels "odd" to have the notification like this, but maybe most people like it.

@poundjd
Copy link

poundjd commented Feb 24, 2018

Some user input, without testing, just my 2 pennies worth (IMNSHO):

  • The notifications slider bar in CIV VI is better when lots of the same or even different types of notifications like you used to get in CIV V, especially when CIV VI "stacks them horizontally with ## of occurrences" ==> very clean looking!
  • The game stores the city pop somewhere, use it if you can catch the delta, before it is stored back there, otherwise store it in your MOD's DB with the city name, or something like that. as the Primary Key.
  • Maybe if you guys wrote a single proper notification module for all of CIV VI's needs, they would use it!
  • Maybe use a parameter or function overloading that specifies what type of notification it is?

Anyway, that's my user input, other than to say THANKS!!!!
-jeff

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants