Project

Profile

Help

HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...

Bug #883354

Small wonders are leaked to all players

Added by Alexandro Ignatiev almost 2 years ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Category:
General
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

We have no interface to see if a player has a specific small wonder unless we see the city. But actually city ids in player.wonders[] are sent to all clients from package_player_common(). Either give some interface to know this important info (I presume it's better) or it is an infoleak that has to be plumbed (like, thousands of players have played for years not knowing about this feature).


Related issues

Related to Freeciv - Feature #850258: Make continent numbers less spoilerificNew

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #923785: wonder_visible_to_player()Closed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Related to Freeciv - Feature #924859: Small Wonder visibility ruleset settingClosed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>

History

#1 Updated by Alexandro Ignatiev almost 2 years ago

Ah I lied there is no interface: we can use Lua function Player:has_wonder() in standard client and iterate over buildings and players. Just not everybody knows Lua, and it seems to me this good leak appeared by mere accident. But more commonly used midclick combat chances function and action enablers also regard all wonders.

#2 Updated by Alexandro Ignatiev almost 2 years ago

I think we should 1) add "Wonders" window to intelligence menu, like,
Palace in Moscow
Second Palace in an unknown city [1234]
Hanging Gardens lost
and 2) add a server option that will either send all wonders as it is done now (all functions will still know them but players also will) or block sending small wonders out of current sight (players won't be able to detect the unseen small wonders, updated client will presume their position from player's map and 6.2 will always think they don't exist and sometimes will show wrong combat chances and attempt illegal actions, or, worse, won't attempt legal ones). In 3.1 we maybe can introduce global visibility for some building flag only.

#3 Updated by Alexandro Ignatiev almost 2 years ago

  • Related to Feature #850258: Make continent numbers less spoilerific added

#4 Updated by Lexxie L over 1 year ago

From multiplayer standpoint, large number of human players has made these rulesets do almost all wonders as Small Wonders.

Ideally, we want to know the Wonder has been built somewhere in the world, and see a count. FCW gives a count of them in the Wonders report. For example: the report might say that 27 Hanging Gardens were built.

Removing ability to know this from Wonders report would "break compatibility" with this functionality.
Still, it's 100% right that there shouldn't be secret packet info with no UI access to it, for obvious reasons.

Any fix to this we'd like to still know the wonders were built and give a global count of them. But there should not be secret packet info about who made them in which city.

An server option to completely repress this info would also be welcome, but hopefully wouldn't break compatibility for rulesets that just want to give a global count of small wonders without revealing which nations and which cities.

#5 Updated by Marko Lindqvist over 1 year ago

  • Tracker changed from Task to Bug

#6 Updated by Marko Lindqvist over 1 year ago

  • Sprint/Milestone changed from 2.6.3 to 2.6.4

#7 Updated by Anonymous about 1 year ago

Alexandro Ignatiev wrote:

2) add a server option that will either send all wonders as it is done now ...
sometimes will show wrong combat chances

Combat chances already depend on data that may be unknown. Obviously for cities (where the combat arguably matters most and where there's a bunch of improvements that affect it). But also e.g. a possible Tech-based defend bonus seems to be silently ignored if you don't have embassy. I.e. the client assumes they don't have the tech, instead of calculating a range or marking the result inconclusive etc. Not that the stock rulesets have that, anyway.

and attempt illegal actions, or, worse, won't attempt legal ones).

Does the client do decisions on actions, though, or does the server do it for the client based on what the client should know?

I played around with just sending "doesn't exist" for all small wonders, (basically just sent zeroes from package_player_common) and didn't really see issues, even if I tried to make the possibility of an action depend on something the client doesn't know. (e.g. depending on a small wonder or regular improvement, that were both invisible now). Would need to look at metaknowledge.c more to make any kind of patch, though.

#8 Updated by Anonymous about 1 year ago

250

Alexandro Ignatiev wrote:

1) add "Wonders" window to intelligence menu, like,
Palace in Moscow
Second Palace in an unknown city [1234]
Hanging Gardens lost

I played around with doing that for the GTK client, see screenshot and patch. This is just a concept, and only for 2.6 and GTK 3.22 now, in case there's anything to change about the interface. I mostly copied the Techs tab and Cities dialog, GTK gurus could check if I effed something up.

Great Wonders in bold, lost or obsolete wonders (as known by viewer) with strikethrough. I think it looks nicer in two columns than
freely flowing. Not sure if it's too noisy with bold and strike sprinkled around, though. Missing translation notes too. Screenshot is from one of the LT rulesets where most wonders are small.

With the GTK client, you can't open the intel dialog without having at least contact though, so this doesn't help with seeing wonders
of faraway nations, even though the server sends them. Maybe make sending them require at least contact? Some would want them
shown only with embassy, though.

and 2) add a server option that will either send all wonders as it is done now ... or block sending small wonders out of current sight (players won't be able to detect the unseen small wonders, updated client will presume their position from player's map

I tried to look at having the server send just the wonders the player has seen, but I can't find a way to iterate over the cities a player remembers seeing. Other than iterating over the whole map, that is. Maybe the client could indeed do it more easily? Iterating over the
real cities would leak information.

#9 Updated by Alexandro Ignatiev about 1 year ago

Ilkka Virta wrote:

and 2) add a server option that will either send all wonders as it is done now ... or block sending small wonders out of current sight (players won't be able to detect the unseen small wonders, updated client will presume their position from player's map

I tried to look at having the server send just the wonders the player has seen, but I can't find a way to iterate over the cities a player remembers seeing. Other than iterating over the whole map, that is. Maybe the client could indeed do it more easily? Iterating over the
real cities would leak information.

Iterate by the wonders, use cached wonder cities of the player, check if it is a great wonder or the other player sees the city now. Cities not seen are the leak, let the player find them on the map themself or by a client-side stuff and guess do they exist and was the wonder rebuilt.

#10 Updated by Lexxie L about 1 year ago

We have no interface to see if a player has a specific small wonder unless we see the city.

Wrong. Some clients have it.

I played around with just sending "doesn't exist" for all small wonders, (basically just sent zeroes from package_player_common) and didn't really see issues,

It could break games/clients/rules that rely on this information.

Multiplayer games of large number of players (usually) rely on a Small Wonder being world-known. It is a big part of game play, diplomacy, and other things.

From our perspective, however, we dislike that theoretically you can hack into client data to cheat and get information that client doesn't display. We consider cheating to be: access any information, ability, or power, that is not commonly available to a non-developer through a user interface.

Our client only wants to display (1) total global number, and (2) wonders for players for whom you can do a legitimate intel report.

#11 Updated by Anonymous about 1 year ago

Alexandro Ignatiev wrote:

Iterate by the wonders, use cached wonder cities of the player, check if it is a great wonder or the other player sees the city now. Cities not seen are the leak, let the player find them on the map themself or by a client-side stuff and guess do they exist and was the wonder rebuilt.

Yes, sending wonders the player actually sees at the moment is easy. But a city might be in view for only a short time, and since
the wonders don't move around very much, it's useful to remember them. They do already show in fogged cities on player's map anyway, so it's not like the information isn't there.

If the server only sends (in packet_player_info) the ones currently seen, the client has to figure out the rest from the map (packet_city[_short]_info). And if the client does that, it doesn't really matter if the server includes the currently-seen ones in player_info or not. It doesn't look too hard to do just that in the client code though.

Lexxie L wrote:

I played around with just sending "doesn't exist" for all small wonders, and didn't really see issues,

It could break games/clients/rules that rely on this information.

The context of that was things like combat chances and actions being impossible. I didn't see any unexpected issues regarding those, or e.g. something crashing. I didn't exhaustively test all possible scenarios, and never said I did.

Multiplayer games of large number of players (usually) rely on a Small Wonder being world-known. It is a big part of game play, diplomacy, and other things.

From our perspective, however, we dislike that theoretically you can hack into client data to cheat and get information that client doesn't display. We consider cheating to be: access any information, ability, or power, that is not commonly available to a non-developer through a user interface.

I'm not sure what you mean. If you want the wonders to be world-known, like the server now appears to make them, what's the possible info leak here? In any case, if there is one such, it should be plugged on the server, so that it wouldn't be possible to abuse it on the client to begin with.

Our client only wants to display (1) total global number, and (2) wonders for players for whom you can do a legitimate intel report.

Total global number should be counted on the server to not leak more details.

#12 Updated by Marko Lindqvist about 1 year ago

I think we need to implement both sides. There should be ruleset control over what information is sent and the gui should show the information in the cases where it's sent.
The ruleset control side can happen in S3_1 in the earliest (older branches are d3f).
Gui changes might get backported to older branches, but generally one should do the development work in development branch (that's the branch where the change will be needed for certain, and often first for testing it before pushing to more stable branches)

#13 Updated by Anonymous about 1 year ago

Marko Lindqvist wrote:

Gui changes might get backported to older branches, but generally one should do the development work in development branch

You've marked this for 2.6, though. Here's the same dialog as in the earlier patch, applied against master as of today.

#14 Updated by Marko Lindqvist about 1 year ago

Ilkka Virta wrote:

Marko Lindqvist wrote:

Gui changes might get backported to older branches, but generally one should do the development work in development branch

You've marked this for 2.6, though. Here's the same dialog as in the earlier patch, applied against master as of today.

What more do you plan to do in the initial version to commit (as you say this is just a 'concept')? From reading the code it seems mostly ready to be the first step, from which follow-up patches can improve.

- There should be empty line between "struct city *pcity = city_from_wonder(p, impr);" and the next code line
- It might be trivial to port this also to gtk3-client (in branches where it exist) and especially in S2_6 it would be beneficial as gtk3-client is the Windows gtk-client

#15 Updated by Anonymous about 1 year ago

Marko Lindqvist wrote:

What more do you plan to do in the initial version to commit (as you say this is just a 'concept')? From reading the code it seems mostly ready to be the first step, from which follow-up patches can improve.

I didn't have any further ideas on the UI.

But, particularly UI stuff can be subjective, and that's just my idea of how it could be done. Others may have different opinions on what the interface should be like. The different branches for the gtk clients are quite numerous, and while I don't expect it to be a problem to apply that dialog to most of them, it's still something that needs to be at least verified manually. I'd rather do that only once, after getting the basic design right. That's what I meant with "in case there's anything to change about the interface".

- There should be empty line between "struct city *pcity = city_from_wonder(p, impr);" and the next code line

Also, it helps to only have to fix this sort of stuff only once. It's not like I leave that kind of stuff in on purpose, mind you, I can just be a bit blind
about it.

- It might be trivial to port this also to gtk3-client (in branches where it exist) and especially in S2_6 it would be beneficial as gtk3-client is the Windows gtk-client

Indeed. I can do that if the dialog is considered ok.

#16 Updated by Marko Lindqvist about 1 year ago

I still have only read the source code, not yet compiled or tested it in game.

Ilkka Virta wrote:

I'd rather do that only once

The main problem that this leaves, like you noted yourself, that it's accessible only when the intel dialog can be opened in general. Whether fixing that in a separate future ticket is against this "only once" wish is mainly your to decide. I just hope that the already existing part does not get postponed too long for waiting the completing part.

#17 Updated by Marko Lindqvist about 1 year ago

#18 Updated by Marko Lindqvist about 1 year ago

Ilkka Virta wrote:

You've marked this for 2.6, though.

The original main issue is present in S2_6. I'm not yet sure what to do with S2_6 & S3_0. It's sort of a rule that the information should not be available, and such a rule should not be changed in a frozen series (especially as this is not about server side dictating same rule for all players) So I'd investigate more if we can stop sending the information, considering it a leak.

For S3_1 and master I'd go for some configurability and the gui. Even without the configurability we can change the rule in those branches so that the gui would be the right thing to do.

So the gui would be needed for master and S3_1, probably not for S2_6 and S3_0.

#19 Updated by Marko Lindqvist about 1 year ago

  • Sprint/Milestone changed from 2.6.4 to 2.6.5

#20 Updated by Lexxie L about 1 year ago

at FCW the intel dialog says the wonders the player has but not the cities. i don't know how we'll eventually sort this whole thing out, but you were addressing client not reporting on what server tells, but there is larger issue too. that it's a little bit too much or should have some more control to it. our issue was not that client doesn't tell what wonders you have, since the intel dialog does that. our issue was that the server tells even more than we want to know, making it possible for some Zekozian/zoltanesque type of player with high skills and no life, to sit there all day intercepting packets to get more info than he should.

so idk, do we open that as a separate issue?

#21 Updated by Marko Lindqvist about 1 year ago

First step towards ruleset control is already in Feature #923785.

Going forward with that sort of depends on this gui stuff. Not much point in telling in the ruleset that the player should know (i.e. the server should sent to client) information that the clients are not showing.

There's still a bit longer route to go before we can provide the functionality that fcw requires, properly implemented. Until then fcw needs to use its own modifications for this part.

#22 Updated by Marko Lindqvist about 1 year ago

In S2_6 & S3_0 client's intel knowledge about capitals of other players depend on small wonders knowledge (in a typical ruleset where Small Wonder determines capital) Making it to work without probably requires bigger changes than one should do in stable branches.
In S3_1 and master client receives pre-calculated information whether a city is capital or not.

#23 Updated by Marko Lindqvist about 1 year ago

  • Related to Feature #924859: Small Wonder visibility ruleset setting added

#24 Updated by Marko Lindqvist about 1 year ago

As we can't limit information sent in S2_6 & S3_0, the gui (this ticket) should go also to those branches -> keeping 2.6.5 target.
Ruleset control (initial version) in Feature #924859
Opened ticket osdn for freeciv-web support: https://osdn.net/projects/freeciv/ticket/41922

#25 Updated by Marko Lindqvist about 1 year ago

Marko Lindqvist wrote:

I still have only read the source code, not yet compiled or tested it in game.

Ilkka Virta wrote:

I'd rather do that only once

The main problem that this leaves, like you noted yourself, that it's accessible only when the intel dialog can be opened in general.

The solution to that one is probably making the dialog available always -> new ticket. That's more general issue than just showing this wonder location information. At least ruler title is shown in some server messages, so the government information should be always accessible.
So the patch has not even that issue. Will you do the porting to other clients & branches?

#26 Updated by Marko Lindqvist 11 months ago

  • Sprint/Milestone changed from 2.6.5 to 2.6.6

#27 Updated by Marko Lindqvist 8 months ago

Marko Lindqvist wrote:

The solution to that one is probably making the dialog available always -> new ticket.

-> https://osdn.net/projects/freeciv/ticket/42917

#28 Updated by Marko Lindqvist 8 months ago

As it has turned out that making intelligence dialog always accessible is not the way to go after all, attached patch (loosely based on Ilkka's one) adds separate dialog for listing wonders.

Retargeting this stuff to S3_0, as it makes no sense to port changes this big to S2_6 this close to it going EOL.

This patch only updates gtk3.22-client. I plan to push it that way. As the default client, gtk3.22-client is the most important one, and I want this finally in for at least that one, not waiting any longer for the other clients. Will open new (osdn) tickets for porting this to other clients.

#29 Updated by Marko Lindqvist 7 months ago

There's an issue with the dialog title, but I'm inclined to go forward with the patch as it is, and leave fixing the title to a new ticket -> https://osdn.net/projects/freeciv/ticket/43043

#30 Updated by Marko Lindqvist 7 months ago

  • Status changed from Resolved to Closed
  • Assignee set to Marko Lindqvist

Also available in: Atom PDF