longturn / freeciv21

Develop your civilization from humble roots to a global empire
GNU General Public License v3.0
223 stars 42 forks source link

Make it possible to forcibly research a tech that is instantly researchable after TC due to tech leak #1753

Closed daavko closed 2 months ago

daavko commented 1 year ago

Is your feature request related to a problem? Please describe. Currently, if RNGeezus wants, a player may end up in a situation where they have more bulbs stored than their current research target costs, due to tech leak. This happens due to player processing order, where a player might gain just-not-enough bulbs to research a tech, but a player processed later might have enough, causing the cost to drop due to tech leak. Sometimes you can then "force" the game to research the tech by switching to another one that's sufficiently expensive (so you don't insta-research it and lose your bulbs) and then switching back, forcing the server to process the research. This may not always be possible, forcing the player to wait until next TC.

Describe the solution you'd like Two potential solutions:

  1. Make the server somehow find out if tech leak would affect players that have already been processed, and process their research again (this might do weird stuff with obsoletions and stuff, I don't know the exact sequence of events in the TC sequence)
  2. Quite a bit simpler, when the player sets their tech goal to the one they're currently researching, the server accepts this and processes it as if the player switched to any other tech. I'm pretty sure this would mean just removing lines 1020-1022 in techtools.cpp, but this is only from a cursory search. It does appear that the client sends the "switch research" packet even when the player clicks on a tech they're currently researching.

Describe alternatives you've considered Leaving it as-is...

Additional context No additional context

lmoureaux commented 1 year ago

Looks like techs are updated first, so obsoletions and so on shouldn't be affected.

lmoureaux commented 1 year ago

I can see two options to take tech leak into account during TC:

Both cases enable a lot of gambling and micromanagement, especially in team games with a lot of info available.

Should getting techs through diplomacy also update other players' research?

jwrober commented 1 year ago

Need to figure out a way to easily reproduce this event as it typically only happens on LT multiplayer games.

jwrober commented 9 months ago

Should getting techs through diplomacy also update other players' research?

My thinking here is that any time a tech is "researched" even if its given in a diplomacy treaty is the same as if the player researched the old fashioned way.

jwrober commented 3 months ago

Re-opening this issue. The PR doesn't do what is expected.

lmoureaux commented 2 months ago

The following check in the server code prevents the player_research packet from refreshing the current tech:

https://github.com/longturn/freeciv21/blob/470a5418b3c10211ed91b127a660854dd8b113f7/server/techtools.cpp#L998-L1000

jwrober commented 2 months ago

Any I'll effects if we remove it?

lmoureaux commented 2 months ago

None that I could find by reading the code, but I haven't tried