Project

General

Profile

Feature #650794

civ2civ3: import some game options from sandbox

Added by David Fernandez (bard) 8 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Category:
Rulesets
Target version:
Start date:
Due date:
% Done:

100%

Estimated time:

Description

- Allow explorers to establish embassies like sandbox, and also to investigate cities. Cost restored from 20 to 30, so they cost the same than diplomats who can also perform those actions. Updated helptext of Explorer.
- Allow those two harmless actions while the actor unit is inside a ship.
- Allow using a caravan to help a team mate build a wonder (same than sandbox).
- Caravan actions require some movement left like other agents, for coherence, as suggested by Sveinung.

- radius_sq_city_permanent changed from 5 to -3 (same than sandbox). If I'm right, it makes the 8 tiles adjacent to the city permanent claimed, and the rest tiles can be stolen by other cities.
Fixed some texts related to achievements.

History

#1 Updated by David Fernandez (bard) 8 months ago

Patch v2 with two agent related effects that I forgot to include:
- No reduced bribe cost of settlers. I think this effect makes diplomats a bit overpowered at the start of the game.
- No Illegal_Action_Move_Cost. I personally don't like it, but there might be some reason for this rule that I do not see.

- Allow explorers to establish embassies like sandbox, and also to investigate cities. Cost restored from 20 to 30, so they cost the same than diplomats who can also perform those actions. Updated helptext of Explorer.
- Allow those two harmless actions while the actor unit is inside a ship.
- Allow using a caravan to help a team mate build a wonder (same than sandbox).
- Caravan actions require some movement left like other agents, for coherence, as suggested by Sveinung.

- radius_sq_city_permanent changed from "-5" to "-3" (same than sandbox). If I'm right, it makes the 8 tiles adjacent to the city permanent claimed, and the rest tiles can be stolen by other cities.
- Fixed some texts related to achievements.

#2 Updated by Sveinung Kvilhaugsvik 8 months ago

  • Status changed from New to In Progress
  • Assignee set to Sveinung Kvilhaugsvik
  • Target version set to 2.6.0
  • % Done changed from 0 to 50

Glad to see you working on civ2civ3 again, David!

Feed back:
I think data/sandbox/README.sandbox needs an update. It talks about how it differs from 2.6 civ2civ3. I split out the border claiming change to Feature #651450 to illustrate what I mean. (Please let me know if the update to civ2civ3's README it includes is correct)

Updated helptext of Explorer.

I think you should warn in the help text that the explorer is consumed by those actions. (I assume that consuming it was your intent)

- No Illegal_Action_Move_Cost. I personally don't like it, but there might be some reason for this rule that I do not see.

The idea is to put a price tag on spamming the server with requests to find out if some action is possible. (This is meant as an explanation, not as an objection)

Fixed some texts related to achievements.

Split out, applied to sandbox too and handled in gna bug #25589

#3 Updated by David Fernandez (bard) 8 months ago

And I'm glad you keep doing this game even more moddable.

I think you should warn in the help text that the explorer is consumed by those actions.

Good point, I copied the text from the diplomat, that seems to lack this info too (at least, I can't find it).

(I assume that consuming it was your intent)

I'd prefer if both diplomats and explorers can survive the investigate city action, but I was unable to get it in v2.6. Please tell me how can I do it, and I'll test it.

(Please let me know if the update to civ2civ3's README it includes is correct)

Done. It seems correct.

The idea is to put a price tag on spamming the server with requests to find out if some action is possible. (This is meant as an explanation, not as an objection)

I'm not confident to change something when I don't know the reason it was there in the first place, so thank you. I still vote to remove it.

Split out, applied to sandbox too and handled in gna bug #25589

As I said there, it seems I copied the improved texts from Experimental, not Sandbox.

Let's pause this patch until we know what to do with the new explorer capability.

#4 Updated by Sveinung Kvilhaugsvik 8 months ago

I still vote to remove it.

OK.

I'd prefer if both diplomats and explorers can survive the investigate city action

I think I understand what you want: At the moment I'm not aware of a way to make a unit survive the investigate city action but not survive establishing an embassy.

Please tell me how can I do it, and I'll test it.

In case I misunderstood: The Spy flag will make the unit survive both establishing an embassy and investigating a city. In that case you would have to test for something else than the Spy flag (and the Diplomat flag) in rules that shouldn't apply to units with those flags. There would also be side effects: the Spy flag would pull inn hard coded Spy rules (diplomatic combat / defense bonus, changes on other actions in the case of the diplomat) and the Diplomat flag.

It's too late to make "dies at the end" more fine grained for 2.6. I have a long term plan that would make it possible. Since I didn't implement this plan in time for 2.6 there is a risk I won't get it done in time for 3.0. I could implement it right now for 3.0 by splitting "Investigate City" in two actions: one where the actor dies at the end and one where it doesn't. That would guarantee that it would be there in 3.0. Would you like me to do that?

#5 Updated by David Fernandez (bard) 7 months ago

I could implement it right now for 3.0 by splitting "Investigate City" in two actions: one where the actor dies at the end and one where it doesn't. That would guarantee that it would be there in 3.0. Would you like me to do that?

That feature would certainly allow what I was trying to do. I'd use it in civ2civ3 and I think it could be useful for other modders too. But there is no hurry, I can wait until 3.1 too.

For now (2.6), I'll make another patch with helptext explaining that explorer is consumed by those actions.

#6 Updated by Sveinung Kvilhaugsvik 7 months ago

I'll make another patch with helptext explaining that explorer is consumed by those actions.

OK. I hope to have time for a review some time next week.

#7 Updated by David Fernandez (bard) 7 months ago

New patch that applies the changes both to civ2civ3 and sandbox, and moves the related readme texts from sandbox to civ2civ3.

From now on, my patches to civ2civ3 will include the same changes for sandbox. If for some reason, we don't want certain change in sandbox, then we replace that part of the patch by a change to sandbox readme.

In this case I was in doubt about the requirement of Writing for the explorer to establish an emabassy in sandbox. In civ2civ3 the diplomats are available with Alphabet (instead of Writing as classic rules), and the diplomats that appear as starting units can perform all the actions without the need of any tech, so I prefer to keep the same behaviour for the explorer. But, if you want me to keep that requirement, tell me and I'll modify the patch.

#8 Updated by Sveinung Kvilhaugsvik 7 months ago

From now on, my patches to civ2civ3 will include the same changes for sandbox. If for some reason, we don't want certain change in sandbox, then we replace that part of the patch by a change to sandbox readme.

Great.

I prefer to keep the same behaviour for the explorer.

I agree. (The rule was ported to the civ2civ3 based sandbox from the classic based experimental)

Issues: * sandbox is missing caravan action movement requirement (remember that sandbox has Enter Marketplace too).

+ without disembarking. Explorers can also perform those two actions, same^M
+ than Diplomats. Cost of Explorers restored to 30.^M * English (I'm not a native speaker): "same than" -> "same as"

Explorers can also perform some actions in another player's city, but\
the unit disappears after the action is complete.\ * English (I'm not a native speaker): "in another player's city". I think "to another player's city" may be more appropriate since the actions are ordered while the unit is outside the city. * Perhaps "another player's city. The" would make the text easier to read for people with low verbal IQ.

+[actionenabler_establish_embassy] * sandbox repeats actionenabler_establish_embassy

#9 Updated by David Fernandez (bard) 7 months ago

New patch with those fixes. Sorry about the duplicated entry... I hate to introduce such game crash causes with my patches.

Changes to help texts are the hardest part for me, I appreciate that someone revises them.
I fixed other "same than" that I found in the readme.
I kept the "in another player's city" because I copied it from Diplomat helptext, and because I did not understand this part:

Perhaps "another player's city. The" would make the text easier to read ...

#10 Updated by Sveinung Kvilhaugsvik 7 months ago

I fixed other "same than" that I found in the readme.

Could we handle that in a separate patch? I don't see how it is related to the rule changes being done here. (I can split it for you if your tools makes splitting a patch hard)

I copied it from Diplomat helptext,

In that case I withdraw my "in vs to" suggestion.

I did not understand this part:

It was two different suggestions. (I'm not used to the way hrm interprets text as formatting yet. This made it hard to read) The suggestion was to split the sentence it in two. So it becomes: "Explorers can also perform some actions in another player's city. The unit disappears after the action is complete."

#11 Updated by David Fernandez (bard) 7 months ago

Could we handle that in a separate patch? I don't see how it is related to the rule changes being done here. (I can split it for you if your tools makes splitting a patch hard)

Yes, please, I use kde linux, and I did not found any integrated gui for svn, so I make the patches with konsole commands, and I do not like very much to work this way.

Btw, I use to make the patch from the freeciv/data/ folder, I'm not sure if you prefer it from the root freeciv/.

#12 Updated by Sveinung Kvilhaugsvik 7 months ago

Yes, please,

Feature #653364

I'm not sure if you prefer it from the root freeciv/.

The freeciv folder as root is better than data as root. A step above freeciv as root (-p1) is even better.

More feed back: it still says "same than Diplomats".

#13 Updated by David Fernandez (bard) 7 months ago

More feed back: it still says "same than Diplomats".

Sorry again, I didn't realized that a text search would not find "same than" when there is a return in the middle of both words.

A step above freeciv as root (-p1) is even better.

My current working copies do not allow that -p1 level (I use svn diff to make the patches).
I'm here in part to learn, so I appreciate suggestions about tools to use or other good practices.

I hope this version is the good one.

#14 Updated by Sveinung Kvilhaugsvik 7 months ago

I'm here in part to learn, so I appreciate suggestions about tools to use or other good practices.

I use git (add, commit, branch, checkout, rebase, am and format-patch) and a text editor (kwrite will do). I know that git rebase has a scary reputation. As long as you send your patches as patch files rather than as git merge requests you won't break anything for anyone else. If you break stuff for your self just go ask for help.

git format-patch + git am + git rebase is extremely useful when splitting a patch. git am has the --keep-cr option for changes to data/civ2civ3/README.civ2civ3

I have attached a git format-patch version of your 2.6 patch to show you how a git format-patch file looks like. You may wish to have a look at it and check the commit message for misunderstandings, errors and omissions. (There is no need for a new patch just to change the commit message so don't worry about delaying anything.)

I hope this version is the good one.

Looks like it is. Ports to 3.0 and 3.1 attached.

#15 Updated by David Fernandez (bard) 7 months ago

I confirm the commit message is right.

It is actually more thorough that the changes to readme that it includes. But the readme is aimed to players, and I think it is better to keep it small when possible.

Thank you for the info, I'll try to change my working copies from svn to git, since it seems freeciv is going to use git in the future anyway.

#16 Updated by Sveinung Kvilhaugsvik 7 months ago

  • Status changed from Resolved to Closed
  • % Done changed from 90 to 100

I'll try to change my working copies from svn to git,

You could use http://repo.or.cz/w/freeciv.git if you don't want to wait for git-svn to import the full freeciv SVN history.

Useful intro to git: https://git-scm.com/book/en/v2

The patch is now committed, by the way.

#17 Updated by Sveinung Kvilhaugsvik 7 months ago

I could implement it right now for 3.0 by splitting "Investigate City" in two actions: one where the actor dies at the end and one where it doesn't.

Was done in hrm Feature #655676.

Also available in: Atom PDF