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

Add UI for blockaded planets #1570

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
7 participants
@TheSilentOne1
Copy link
Member

TheSilentOne1 commented May 15, 2017

Adds an icon over planets that are currently being blockaded:
blockade

Icon art is placeholder/WIP.

Some things I'd appreciate input on:

  • What exactly are the criteria for a planet being blockaded? A planet is blockaded if an armed hostile fleet is present at the system, but is it also blocked if friendly ships are around? And if a planet is invisible, will it still be blockaded?
  • Is there some central piece of code that handles planet blockade? Since I couldn't find any, I added bool PlanetBlockaded(int planet_id) to Sidepanel.cpp, but I'm not sure if it's in the right place there. Maybe rather in planet.cpp bool Planet->Blockaded()?
  • and lastly, how do I assess monster detection strength? Is there a better way than to cycle through the individual monster ships and look for the best detection strength?
@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 15, 2017

What exactly are the criteria for a planet being blockaded? ... Is there some central piece of code that handles planet blockade? Since I couldn't find any...

Is there some forum discussion of what you are trying to accomplish here? Are you sure you really want to tie this to planets being blockaded, versus just calling more attention to when a system is under attack regardless of whether that really results in a blockade?

Regarding your questions:
What git tool do you use on your local machine; does it have any way to search the log? I use SmartGit, and if I search for 'blockade' within the log, then it comes up with just 7 commits. For the explanation of blockades, see the the changelog explanation in 9be49c1. (It looks like perhaps we have overlooked also getting that into the Pedia, which should be rectified). There are two aspects to blockades-- fleet movement and supply blocks. It seems this is focused on the latter, but I suggest you start by looking at how the mapwnd decides to show blockades for fleet paths, which you can at least mostly find in 6345b20. Since your focus is supply blocks may want to also look at the actual supply block determination in Empire::UpdateSupplyUnobstructedSystems (5039f12 can help guide you), but avoid duplicating the supply block determination code (at worst, refactor things to extract some part you might need to use for this new UI element).

@dbenage-cx

This comment has been minimized.

Copy link
Member

dbenage-cx commented May 15, 2017

It looks like perhaps we have overlooked also getting that into the Pedia, which should be rectified

Linking #1454 in case the mentioned commits are helpful with some future article.

@Vezzra Vezzra added this to the v0.4.8 milestone May 16, 2017

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 16, 2017

@Dilvish-fo: Some discussion is here: http://www.freeorion.org/forum/viewtopic.php?f=28&p=88798. Note that I don't intend to change any game mechanics. What I'm trying to improve is how blockades and their effects (disconnection of planets from industry pool, no regeneration of ground troops) are presented to the player. Right now the player does not get any indication that troop regeneration is stopped.
It's good that you point out that blockades affect the whole system and not individual planets. The "Blockade UI element" could maybe be shown on top of the sidepanel, on the system summary, like "all your planets in this System are blockaded". That would certainly save some space.

...duplicating the supply block determination code (at worst, refactor things to extract some part you might need to use for this new UI element).

I'll put some thought in that and discuss my ideas here.

@dbenage-cx: Thanks for linking the issue. I'll take it on while I'm on this.

@TheSilentOne1 TheSilentOne1 self-assigned this May 16, 2017

@LGM-Doyle

This comment has been minimized.

Copy link
Contributor

LGM-Doyle commented May 16, 2017

@TheSilentOne1 I like the idea and the icon.

Feature Creep Alert

Since it is system wide, do you think it is a good idea to add it as an underlay to the system icon on the MapWnd? Or is this too much clutter?

@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented May 16, 2017

Should the icon only show the player's planets, or should there be an indicator if other empires' planets in the same system are blockaded? Could be multiple other empires, so a per-planet indicator might be useful... and would probably be easier to see / harder to not notice in any case.

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented May 16, 2017

I think @geoffthemedio has a point here.

A visual indicator on the map for systems with currently blockaded planets sounds useful too.

@LGM-Doyle

This comment has been minimized.

Copy link
Contributor

LGM-Doyle commented May 16, 2017

I was thinking that the MapWnd indicator be in addition to the per planet indicator already in this PR.

I think that as the blockading mechanic currently works, there can only be one blockading empire per system, the empire that first established a stationary fleet in the system to secure the blockade.

On the MapWnd the blockade indicator shows that there is a blockade in the system. If there is only one fleet in orbit then that indication is unambiguous, because you only blockade hostile planets. If there are multiple fleets in orbit, then it is ambiguous.

In the SidePanel it is unambiguous, because only planets hostile to the blockading fleet have the blockade indicator.

A way at the MapWnd level to indicator which empire is blockading might be to tint the MapWnd blockade indicator with the blockading empire's color. The tinting could also be applied to the SidePanel indicator for some uniformity of interface.

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented May 16, 2017

Agree with everything @LGM-Doyle said.

@geoffthemedio

This comment has been minimized.

Copy link
Member

geoffthemedio commented May 16, 2017

I think it can be a bit more complicated...

There could be 4 empires, A, B, C, and D. If A and B have planets in the system, and are respectively at war only with C and D, then D is blockading B and C is blockading A, but no-one else is blockading anyone.

Or does the presence of neutral (armed) ships in a system disrupt a blockade?

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented May 16, 2017

Or does the presence of neutral (armed) ships in a system disrupt a blockade?

I think not, at least it shouldn't be the case. What does the code say?

@Vezzra Vezzra changed the title add UI for blockaded planets (WIP) Add UI for blockaded planets May 16, 2017

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented May 16, 2017

@TheSilentOne1: We have a label to indicate "work in progress", please use that instead of indicating this with "(WIP)" in the PR title. 😉

Edited the title and added the label.

@LGM-Doyle

This comment has been minimized.

Copy link
Contributor

LGM-Doyle commented May 16, 2017

Neutral fleets don't affect the blockade.

Here are two possible solutions.

One solution would be to split the tint into pie sections for each fleet assisting in the blockade. Then it does not matter which empire is blockading which planet in terms of the UI.

Another solution is to use the current code mechanic, time of first arrival to determine which empire is blockading the system and each planet.

For the system, there is only 1 fleet blockading the system, the fleet that first arrived and established the blockade. Its allies are assisting in the blockade. For the example, say empire C's fleet arrived first and so holds the blockade. Use C's tint for the system indicator.

Then at the planet level use the same mechanic. The earliest arriving fleet participating in the blockade hostile to a given colony determines the color of the blockade marker. For the example the colors are unambiguous, since there is only one hostile empire per colony. So, use C's color to tint A's colony's blockade marker and use D's color for B's colony.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 16, 2017

Everyone keeps referring to "blockade" but a major aspect of the desired result (the part with the least current UI representation) is quite distinct from blockades:

Right now the player does not get any indication that troop regeneration is stopped.

Troop regen is determined entirely by the last time of local battle, totally independent of whether a blockade was established or even whether the enemy had any survivors:

CANDIDATE_BATTLE_CHECK
'''Turn low = LocalCandidate.System.LastTurnBattleHere + 1'''
@MatGB

This comment has been minimized.

Copy link
Member

MatGB commented May 16, 2017

I think the problem is that there are two distinct things that're described as a blockade, which causes some confusion.

Both systems and planets are blockaded when there's an aggressive enemy fleet stationary and there are no friendlies to defend, the blockade continues until the friendlies defeat the enemy or the enemy moves away. But a system being blockaded and a planet being blockaded are different. One already has a UI indication if supply is being blocked, but the sidepanel doesn't and it's that that needs sorting.

I had, mistakenly, thought that it was the blockade that prevented troop growth not a script relating to combat and that does mean that it can't be combined: you could send in a cheap cannon fodder ship each turn to prevent troop regen but not actually win against a doomstack, I hadn't actually noticed that before. However, the mouseover should show no growth not as it currently does:
spectacle xm4423
Hulse has been blockaded by my fleet for a few turns now and I've landed some troops.

It would be very useful for players, especially in ProdWnd, to have some indication that a system that isn't using all its PP has each individual planet blocked from sharing them, etc: I sometimes miss this so less experienced players definitely will: but there is a difference between 'combat last turn' which affects things like Drydocks and troop regen, and blockade (picket?) which affects supply propagation and resource sharing.

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented May 18, 2017

Troop regen is determined entirely by the last time of local battle, totally independent of whether a blockade was established or even whether the enemy had any survivors

you could send in a cheap cannon fodder ship each turn to prevent troop regen but not actually win against a doomstack

I consider this a bug/exploit, which needs to be fixed. My suggestion: prevent troop regen only when agressive armed enemy forces are present in the system.

However, the mouseover should show no growth not as it currently does

That should also be fixed. Showing a predicted growth that isn't going to happen is confusing for the player.

We should open issues for both cases - any objections?

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 18, 2017

We have a label to indicate "work in progress", please use that instead of indicating this with "(WIP)" in the PR title. 😉

Right, thanks.

My suggestion: prevent troop regen only when agressive armed enemy forces are present in the system.

That should also be fixed. Showing a predicted growth that isn't going to happen is confusing for the player.

I agree on both points.

Now one thing that's still unclear to me: if two hostile empires have armed fleets in the same system, will planets be blockaded or not? Does a friendly fleet counter a blockade?

Also, we should think about a UI for "secure" and "unsecured" starlanes. I hadn't even know this feature existed before reading through this 9be49c1.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 18, 2017

Now one thing that's still unclear to me: if two hostile empires have armed fleets in the same system, will planets be blockaded or not? Does a friendly fleet counter a blockade?

Those questions do not have simple yes-no answers; the answer depends on other factors. It looks like you read the writeup in the changelog, if that's not enough then try the explanation that dbenage-cx (indirectly) linked to.

Also, we should think about a UI for "secure" and "unsecured" starlanes. I hadn't even know this feature existed before reading through this 9be49c1.

There is already a UI representation of this-- the type of fleetpath waypoint markers used if you check fleet paths for departing from the system (and the eta count on the markers, if the next system destination was also blockaded). The only time it matters is when you are looking to have a fleet move, so I think that is sufficient representation.

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 19, 2017

It looks like you read the writeup in the changelog, if that's not enough then try the explanation that dbenage-cx (indirectly) linked to.

Thanks for linking this post. I've written up the information (hopefully correctly) for a pedia entry and created a PR here: #1579.

The only time it matters is when you are looking to have a fleet move, so I think that is sufficient representation.

I think that may be too subtle, because I was never aware how this worked. But I'll test it in game with my newly-aquired knowledge.

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 19, 2017

It seems to me that the UI / eta marker is misleading, it currently seems to suggest that you will be able to move your fleet through a blockaded system on the cost of one turn and one forced battle (which you can't). Instead the UI should show that you can't move out of a system (except for the lane by which the fleet entered) if a blockade is in place.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 19, 2017

t seems to me that the UI / eta marker is misleading, it currently seems to suggest that you will be able to move your fleet through a blockaded system on the cost of one turn and one forced battle (which you can't).

I believe the current UI ETA marker is correct within its reasonable limitations-- it shows you what the result will be if you win the battle at the blockaded system or if the system is being blockaded by a monster. For a non-monster blockader, any delay in winning the battle would add corresponding delay to the ETA, but the UI can't really predict that. This is all helpful information for the player (but granted, we need to make sure we get a sufficient explanation into the Pedia).

Instead the UI should show that you can't move out of a system (except for the lane by which the fleet entered) if a blockade is in place.

This sounds to me just like an unhelpful removal of information.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 19, 2017

@TheSilentOne1 For reference, here is the discussion where we settled on most or all of the UI elements for the current blockade system. Geoff began with at least some similar concerns as you (albeit I think his concerns were more about a somewhat earlier iteration). It should at least help clarify why we arrived at the current blockade display.

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 20, 2017

For a non-monster blockader, any delay in winning the battle would add corresponding delay to the ETA, but the UI can't really predict that.

So maybe we could add an indication of this uncertainty, for example by adding an additional eta marker at the exit with an "x" or "?". This indicates that there's an obstacle leaving the system, that may take an unpredicatable amount of time until it is removed.

additional_marker

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 20, 2017

The ones in red, like in your picture, means that they are at or after an expected battle, so on any of them after the first one, the eta is uncertain. I don't recall at the moment if there is any difference other than the color-- I think I recall yandonman raising the issue that we would want the distinction to be workable for color-blind people, but I don't recall if we resolved that or not. I am not noticing a distincton right now between the red and yellow blockade markers besides the color. Did you come across that part of the discussion? I suppose replacing that first combat ETA number with an 'X' like you suggest would make it distinguishable for the colorblind.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 20, 2017

Also, it just struck me that since the meanings of our ETA/blockade path waypoints are apparently not intuitively obvious to everyone, it would seem helpful to have something about them added to the Guides/Interface/FleetMovement article.

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 20, 2017

I suppose replacing that first combat ETA number with an 'X' like you suggest would make it distinguishable for the colorblind.

Currently, this marker isn't there, I added it on the starlane exit. That's how I would indicate there is a stop there.

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 20, 2017

Currently, this marker isn't there, I added it on the starlane exit. That's how I would indicate there is a stop there.

Ah, ok. I have no objection to that representation, and I think in at least some cases it would not be redundant and would add clarity (where the "2" ETA marker is right at the entrance to a system that is itself blockaded and is at a distance very close to the fleet's single turn travel distance, so it might not be clear if there could be "1" ETA marker very slightly prior to the "2" marker but effectively hidden)

@dbenage-cx

This comment has been minimized.

Copy link
Member

dbenage-cx commented May 21, 2017

Troop regen is determined entirely by the last time of local battle, totally independent of whether a blockade was established or even whether the enemy had any survivors
you could send in a cheap cannon fodder ship each turn to prevent troop regen but not actually win against a doomstack

I consider this a bug/exploit, which needs to be fixed. My suggestion: prevent troop regen only when agressive armed enemy forces are present in the system.

Might take that a little further to: prevent planet meter growth if there is an aggressive enemy ship with direct weapons in-system (can attack planet).

Which empire color is chosen when there are non-visible ships that blockade supply? (from stealth or lack of detection range)

@Dilvish-fo

This comment has been minimized.

Copy link
Member

Dilvish-fo commented May 21, 2017

Might take that a little further to: prevent planet meter growth if there is an aggressive enemy ship with direct weapons in-system (can attack planet).

It's already extremely close to that -- there is no general meter growth if the planet has actually been attacked. For the general meters at least I personally prefer that current treatment instead of denying meter growth merely because of the threat of attack.

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented May 24, 2017

If there's no objections I'll close this for now. I'll do some concepts for the blockade UI and put them up for discussion on the forums. Discussion about when to suspend meter growth can continue there.

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented Jun 5, 2017

Closed as "follow up", I assume, as apparently you intend to provide a revised implementation? Just need to know which labels/milestone to set...

@TheSilentOne1 TheSilentOne1 modified the milestones: Undecided/TBD, v0.4.8 Jun 6, 2017

@TheSilentOne1

This comment has been minimized.

Copy link
Member Author

TheSilentOne1 commented Jun 6, 2017

Yes, that's correct - it may be a while though. I've set the labels.

@Vezzra

This comment has been minimized.

Copy link
Member

Vezzra commented Jun 11, 2017

Ok, once the follow up PR has been created, please post a reference to it here, clear the "work in progress" label and set the milestone to "Gateway to the Void".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.