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...

Feature #859061

Ruleset defined counters

Added by Sveinung Kvilhaugsvik over 2 years ago. Updated 3 months ago.

Status:
New
Priority:
Normal
Assignee:
-
Category:
General
Sprint/Milestone:
Start date:
Due date:
% Done:

0%

Estimated time:

Description

I got this idea today. I haven't thought it through yet. If this ends up being implemented it may end up looking quite different,

Pre requirements

Use requirements to check counter values.
Example:
CounterGreaterThanZero requirement type: Feature #859052
Alternative: MinCounterValue requirement type with both counter and value would make CounterGreaterThanZero redundant. Could maybe be done by putting both in the name field separated by a special symbol or by extending the requirement syntax.

Use effects to modify counter values.
Use the new requirement type CounterType to specify counter to be modified.
Examples:
CityEndTurnCounterReset - reset the city's counters (selected with CounterType) to the effect value at end of the turn. Could this replace the "Airlift" effect?
CityEndTurnCounterAdd - add the effect value to the city's counters (selected with CounterType) at end of the turn.
Action_Success_Actor_Counter_Add - adds the (probably negative) effect value once an action is successfully completed. Alternative: name the effect ...Cost and subtract the (probably positive) effect value.

Main part

Define counters in the ruleset
Define name and location. Location can be city, unit, player etc.
Should probably define if permanent or reset each turn. (Don't do reset effects on non resetable counters)
Maybe define value kind (float, int), step, min value and max value too?

0001-Added-structures-for-counters-into-multipliers.h.patch (1.29 KB) 0001-Added-structures-for-counters-into-multipliers.h.patch Sławomir Lach, 2020-03-23 10:32 AM
0001-Move-some-routines-from-progress-project-to-multipli.patch (2.14 KB) 0001-Move-some-routines-from-progress-project-to-multipli.patch Sławomir Lach, 2020-03-23 10:53 AM
0001-Added-variable-range-support.patch (14.5 KB) 0001-Added-variable-range-support.patch Sławomir Lach, 2020-03-24 03:24 PM
0001-Adding-counters-completed.patch (50.5 KB) 0001-Adding-counters-completed.patch Sławomir Lach, 2020-03-26 05:11 PM
0001-Working-ruleset-which-implementing-golden-age.patch (3.71 KB) 0001-Working-ruleset-which-implementing-golden-age.patch Sławomir Lach, 2020-03-26 05:13 PM
0001-Properly-freeing-a-memory.patch (1.8 KB) 0001-Properly-freeing-a-memory.patch Sławomir Lach, 2020-03-27 10:10 AM
0001-Added-initial-counter-types-declarations.patch (1.95 KB) 0001-Added-initial-counter-types-declarations.patch Initial strcutures. Sławomir Lach, 2020-04-10 07:13 PM
0001-Added-initial-counters-implementation-with-initial-c.patch (8.36 KB) 0001-Added-initial-counters-implementation-with-initial-c.patch Sławomir Lach, 2020-04-11 09:48 AM
0005-Repair-misspeled-inplace-of.patch (24.5 KB) 0005-Repair-misspeled-inplace-of.patch Sławomir Lach, 2020-04-13 02:34 PM
0006-Introduce-priorities.patch (8.2 KB) 0006-Introduce-priorities.patch Sławomir Lach, 2020-04-13 02:34 PM
0001-Ruleset-changes.patch (1.81 KB) 0001-Ruleset-changes.patch Sławomir Lach, 2020-04-13 02:38 PM
0001-Check-content-of-counters.ruleset-file-during-rulese.patch (4.82 KB) 0001-Check-content-of-counters.ruleset-file-during-rulese.patch Sławomir Lach, 2020-04-14 12:26 PM
0002-Added-code-responsible-for-freeing-up-memory.patch (3.36 KB) 0002-Added-code-responsible-for-freeing-up-memory.patch Sławomir Lach, 2020-04-14 12:27 PM
0003-Introduce-unit-and-city-counters.patch (7.21 KB) 0003-Introduce-unit-and-city-counters.patch Sławomir Lach, 2020-04-14 12:27 PM
0004-Added-test-for-city-counters.patch (1.59 KB) 0004-Added-test-for-city-counters.patch Sławomir Lach, 2020-04-14 12:27 PM
0003-Rework-datatypes.patch (2.1 KB) 0003-Rework-datatypes.patch Sławomir Lach, 2020-04-14 12:28 PM
0004-Added-code-to-load-counters.patch (9.81 KB) 0004-Added-code-to-load-counters.patch Sławomir Lach, 2020-04-14 12:28 PM
0005-Changes-in-structures.patch (16.4 KB) 0005-Changes-in-structures.patch Sławomir Lach, 2020-04-14 12:29 PM
0001-Added-counter-which-makes-units-killed-at-least-4-ot.patch (1.28 KB) 0001-Added-counter-which-makes-units-killed-at-least-4-ot.patch Sławomir Lach, 2020-04-14 12:49 PM
0001-Repair-spelling-error-I-copy-code-and-not-change-poi.patch (2.15 KB) 0001-Repair-spelling-error-I-copy-code-and-not-change-poi.patch Sławomir Lach, 2020-04-14 02:09 PM
0001-Changing-signal-because-kill-is-relate-to-object.-I-.patch (843 Bytes) 0001-Changing-signal-because-kill-is-relate-to-object.-I-.patch Sławomir Lach, 2020-04-14 02:15 PM
0001-Added-support-for-world-counter.patch (16.4 KB) 0001-Added-support-for-world-counter.patch Sławomir Lach, 2020-04-15 09:20 AM
0001-Added-example-ruleset-change-to-demonstrate-world-co.patch (3.01 KB) 0001-Added-example-ruleset-change-to-demonstrate-world-co.patch Sławomir Lach, 2020-04-15 09:20 AM
archive.tar.gz (24 KB) archive.tar.gz Sławomir Lach, 2020-04-23 11:56 AM

Related issues

Related to Freeciv - Feature #876776: Counters' primary structures.Closed

<a title="Actions" class="icon-only icon-actions js-contextmenu" href="#">Actions</a>
Blocked by Freeciv - Feature #859052: New requirement type: CounterGreaterThanZeroNew

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

History

#1 Updated by Sveinung Kvilhaugsvik over 2 years ago

  • Blocked by Feature #859052: New requirement type: CounterGreaterThanZero added

#2 Updated by Alexandro Ignatiev over 2 years ago

I think data types other than short int and maybe a cycled enum are redundant here, on severe need one can just use Lua associated values like data[find.unit(plr,id)].mycounter = 3.1415 in callbacks. Probably first concentrate on city and/or player counters, units are usually OK with their fixed structure (e.g. if a missionare evangelizes 4 cities max., just reduce his health 25% on an action and don't restore).

#3 Updated by Alexandro Ignatiev over 2 years ago

We probably need counters for tiles (like, Civ4 growong cottages, or a realistic forest that can be planted by any number of workers but then you can just wait some turns/game years until it becomes economically useable). Maybe you can get fine with a single counter per tile, or maybe you also put some "fortress health" into this system.

#4 Updated by Alexandro Ignatiev over 2 years ago

And the most probable thing is unifying the counters system with the multipliers system, because why not. Like, some counters are set indirectly by game rules and the another directly by players.

#5 Updated by Sławomir Lach over 2 years ago

http://forum.freeciv.org/f/viewtopic.php?f=11&t=91348&start=10
sveinung asked me to integrate changes in Progress related to this topic to Freeciv. I attached first patch. Changes was in multipliers, because progress introduces also restrictions, which would be way to restrict some arbitrary values (i split some part of multipliers into a new entity, called restriction), but currently are used only to restrict multipliers. Counters are also in multipliers, cause it could be used by restrictions to restrict values of multipliers.

I will describe my way to handle counters. We have had two structures. First describes counter type. It contains info such like: name (translated, rule, etc.), default value and range (player, unit, city, world). Progress currently only handles world and player counters. Second is used to calculations. It have two fields: current and future (in progress I use new as a name, but I suppose It's wrong for Freeciv, because Qt client need to be compiled by c++ compiler). Each counter's future value could be calculated from any other counter's current value. After new turn begins, we updates current values from futures and do calculation of future values. So why to store both values? Because counters could been used such like multipliers -- to multiply value of effects, so we use current value for effect multiplication and future field is used by effects operating on counters value.

Further of this patch (I must discuss whole idea before sending more changes), we have special array of functions comparing two values, which could been used in requirement list. We have also requirements to give an local name to counter. We have also functions to do calculation on local names. But ... I made many changes in requirements.h/requirements.c, so we use array of local req_list variables, so these functions could work.

#6 Updated by Sławomir Lach over 2 years ago

To more clarify, why I use future and current.
Imagine we didn't calculate counter value on turn start or doesn't split value into future and current.
1) When calculating values of counters, result of calculating one could have impact on calculating other counter (how to determine order of calculation?)
2) Imagine some requirement changes or effects was calculated many times on turn - how to predict value of counter?

#7 Updated by Sławomir Lach over 2 years ago

This are part of multiplier.c file. Probably full code related to counters from multipiler.c are fulfilled in this patch.
Sorry for size of this patch, sending one patch not far from other, but I was asked to split changes into small part of code, so I do this.

I must comment some part of code and replace values in loop with false, because it won't compile in other way. Next part will be split code related to counters from ruleset.c, but I will wait for suggestions now.

#8 Updated by Sławomir Lach over 2 years ago

I sending a new patch, with allow to load counters and add requirement to rulesetdir to contain counters.ruleset file.
I probably will spend one to three days from tomorrow to end move my work.

#9 Updated by Sławomir Lach over 2 years ago

The last changes to moving implementation of player-related counters from Progress game to Freeciv game. I can also implement world counters, but implementing cities/units will need more time. World counters I could implement tomorrow. And about tomorrow - I will apply suggestion about code formatting/quality.

Everything seems to works.

#12 Updated by Alexandro Ignatiev over 2 years ago

Sławomir, I appreciate your work! But I doubt the syntax for rulesets is good. The principle that each requirement stays for its own is just good, clear and AI code likely relies on it a lot. If I were as bold as you I would try implementing it like this:

;Gives 3 turns of +200%prod
;then 10 normal turns
[counter_happiness]
name="TurnsBeforeGA" 
range="Player" 
default=2
min=0
event="turn_end" 
step=-1
step_reqs={"type", "name", "range" 
 "Building", "Palace", "Player" 
}
set=10
set_reqs={"type", "name", "range", "present" 
 "CounterAboveZero", "TurnsOfGA", "Player", FALSE
 "CounterAboveZero", "TurnsBeforeGA", "Player", FALSE
}

[counter_turns_of_ga]
name="TurnsOfGA" 
range="Player" 
default=0
min=0
event="turn_begin" 
set=3
set_reqs={"type", "name", "range", "present" 
 "CounterAboveZero", "TurnsBeforeGA", "Player", FALSE
}
step=-1
;;;;;in effects.ruleset;;;;;
[effect_durning_golden]
type = "Output_Bonus" 
value = 200
reqs = 
{
  "type", "name", "range" 
  "CounterAboveZero", "TurnsOfGA", "Player" 
}

Of course this is a tiny syntax, more complex tasks would require several other features but still something that ruleset authors, players and AI can understand more or less at ease.

Btw, maybe counter most pleading moving to ruleset is DiplRel-ranged CasusBelli (and maybe somehow accumulated AI_Love); Civ>=3 use WarWeariness counter instead. If a multiplier is an optional interface to a counters, DiplRel-based ones may be adjusted by agreements (so we do get annual tribute/luxury trade).

#13 Updated by Sławomir Lach over 2 years ago

I propose other solution:
Move CounterToName requirementsw to special section called „imports”. It could also have less memory usage.

Another thing, implemented in Progress game, is that we could use counters as multipliers of effect.

#14 Updated by Alexandro Ignatiev over 2 years ago

"Interface" I mean

       [Player]   [Player]
        |     \      |
[Multiplier]  [Agreement]
        |           |
[Player-range]  [DiplRel-range]
[  counter   ]  [   counter   ]

I.e. multiplier describes the UI appearance of the counter, factor and offset, and reqs when player can change it, and the counter is the thing actually working in the game. Resetting value to default when player loses control may be unhardcoded to the counter as a set-triggering event. The changing interface is of course optional, we probably should design also vision range of counters: to what clients any value changes are reported.

#15 Updated by Sławomir Lach over 2 years ago

I have one week free of work (in worst case) and partially done other projects. I decided to start work at user-definable counters from begginning.

To check I understood how counters should works, I post initial data types.

#16 Updated by Alexandro Ignatiev over 2 years ago

Stylistic moaning: Maybe we can omit the convention "counter_type"? Yes, actually we have different counters for different object that work by similar rules defined in this structure, but we could save a lot of typing "_type" calling it just "counter" and each of the instances "counter value". Also, "counter type" looks better for speaking about player counters, world counters etc. then "counter range" because "range" also means "diapasone". But of course you are the main man who types it now and you decide what is more comfortable for you to think about.

Probably, when we load the ruleset, we put all counter types<counters> of the same range<type> in arrays and add the counters<counter values> to the game objects as corresponding arrays (if I got it well, player.plr_counters[] in Postęp works like this). The counter<counter value> type may then be just int (or struct{int, int} if we keep "value, old value" style, but binding counters to events when they update basically eliminate the necessity to store "new" values, client may calculate them itself when the user examines where the things are going to).

Then, I think even on this early stage we should be ready that counter may change on more than one event (e.g., war weariness in Civ3 is changed in unit-on-unit actions, unit kills, city captures, diplomatic meetings etc.). In the notation I used before, I supposed to have it as

event = ...
set = ...
set_reqs = ...
event1 = ...
step1 = ...
step_reqs1 = ...

Thus the counter_type<counter> struct must have some struct counter_event_list events; listing something like
struct counter_event {
  const char *event; /* maybe we could use an index for events? */
  struct counter_type/*<counter>*/ counter; /* backlink */
  struct {
    char player, other_player, city, building, tile, unit, unittype, specialist, action;
  } args;
  char self;
  struct event_constraint_list constraints;
  int set, step;
  struct requirement_vector *reqs, *step_reqs;
};

Of course one can design it with enum counter_effect_type with only one value and reqs for each operation.

args are numbers of luascript_signal_emit() parameters that we pass to are_reqs_active(). By default they are filled by first event parameters of appropriate enum api_types defined in luascript_signal_create() but we may want to have syntax to redefine something. The first parameter of the type matching the counter's range is "self" for which the counter is stored, this also may need a syntax to redefine.

event_constraint_list is something with which we filter event parameters that we don't have requirements for, e.g., that unit_lost callback 3rd parameter is string "killed". Maybe just a precompiled string of Lua code would do the job well. Like,

[counter_cities_conq_embassy]
name = "CitiesConqueredWithEmbassy" 
range = "Player" 
event = "city_transferred" ; (City city, Player loser, Player winner, String reason)
constraints = $ return arg[4] == "conquest" $
params = {"type", "number", "self" 
  "City", 1
  "Player", 3, TRUE
  "OtherPlayer", 2}
step = 1
step_reqs = {"type", "name", "range" 
  "DiplReL", "Has Embassy", "Local" 
}

Maybe we want to use multipliers/divisors to step and set value. E.g. Civ3: city has accumulated unhappiness: each unhappy event like defender has lost or population starved increases accumulator on some amount, then the effect takes away (int) (accumuator + option(unhappy_turns) - 1) / option(unhappy_turns) happy faces while accumulator falls per turn on the same value.

[counter_city_unhappiness]
name = "CityUnhappiness" 
range = "City" 
event = "phase_end" 
step = -1 ; 
step_multiplier = "CityUnhappiness" 
step_divisor = "unhappy_turns" 
event1 = "city_size_change" 
constraints1 = $ return arg[2] < 0 $ ;the change; we'd better multiply on it but hardly
step1 = 1
step_multiplier1 = "unhappy_turns" 
;...

But the AI will have hard time understanding what to do about it...

Maybe as a further extension of the mechanism we also allow supplying named diapasones for counters like

[counter_repute]
name = "Repute" 
range = "Player" 
default = 0
diapasones = {"name", "min", "max" 
  "Bad", , -1
  "Moderate", 0, 99
  "Good", 100, 999
  "Excelllent", 1000}
;some good and bad deed events

[effect_trade_good_repute]
type = "Trade_Revenue_Bonus" 
value = 500
reqs = {"type", "name", "range" 
  "CounterDiapasone", "Repute.Good", "Player" 
}

but AI will have more troubles to get into a desired diapasone then just to keep the counter above or below zero; maybe we should also make math with counters powered by counters.

How AI should assess it: we define a function adv_want assess_event_counters(const eft_callback ecal, const char *event_name, ...) where ecal is a function pointer that specifies the want the specific AI algorithm puts into each effect. This function is called before AI does something tha may trgger an event with the event's name and parameters. There is a list of all counter types<counters> that the event changes, and for each counter type<counter> there is an effect list that it modifies as multiplier or as a requirement ("above zero", at this stage). As a requirement, it may modify many other things that may need another callbacks though... among them it may modify other counters that requires some recursion and maybe squaring... but finally the want is computed and returned to the AI that adds it to the action want and looks if it finally senses good.

#17 Updated by Sławomir Lach over 2 years ago

I send this cause good programmers should do commits as often as possible.

#18 Updated by Sławomir Lach over 2 years ago

I have an problem with signals. Is there exist a signal called turn_end?

#19 Updated by Alexandro Ignatiev over 2 years ago

Sławomir Lach wrote:

I have an problem with signals. Is there exist a signal called turn_end?

Currently no, nobody has defind it, all that exist are here.

Yes, probably we can avoid multiplying counters for city unhapiness, we'd better define third counter event scale_pml (e.g. for 20 city unhappy turns scale_pml = 950).

#20 Updated by Sławomir Lach over 2 years ago

Sorry for I didn't tidy in code and in path.

Currently It's only proof of concept. Counter given in example worked. I must add one line to example, but that's probably Alexandro's mistake.
I send ruleset changes in next comment.

Only player counters worked and some feature still missing. I will work on this feature, so be patient and help me as far as you can.

#21 Updated by Sławomir Lach over 2 years ago

Corrected ruleset.

I introduces priorities. Higher priority (integer value) means effect takes precedence.

#23 Updated by Sławomir Lach over 2 years ago

These patches introduces city/unit counters. I test only city counters by adding health variable. When health of city is equal to 0, each output is reduced by half.

Granary will increase health, while factory reduces. And yet one thing. City with minimal size of 2 have reduced health by 1, so if you don't build granary, production of food/shields/gold is reduced by 50%, cause health equals 0.

#26 Updated by Sławomir Lach over 2 years ago

This counter made unit that killed at least 4 other units healing 80% of it's health per turn.

#27 Updated by Sławomir Lach over 2 years ago

Sorry for messing up in this thread. I introduce bug with new features (this bug was related to new features). Missing feature (not checking requirement) with spelling error makes me thinking everything worked. Later I realize each unit heals 80%. This bug was repaired. I start working on this feature tomorrow, so comment, please.

#29 Updated by Sławomir Lach over 2 years ago

I made few, smaller changes. Self range allowing to force to iterate over objects of specific range (value will be stored on normal object, but it update value on each iteration). It is helpful for environment victory (thing I do in new ruleset).

#31 Updated by Alexandro Ignatiev over 2 years ago

If I see clear, you have not published the patch which introduces self_range and other iterating params, so I can't comment how they work. Also, could not find the code for C_signal_hander_vector_sort() and what do priorities do (could we somehow get around manually enumerating things?). My own idea was to make more events (like "city_processed" when we update each one of a player's cities at phase end) but yours with iterators looks even more flexible and not much more complicated. Also, I presumed that counters are updated for the same event in their declaration order (maybe firstly for more narrow ranges?) but this of course needs better thinking about and testing.

#32 Updated by Sławomir Lach over 2 years ago

0001-Added-support-for-world-counter.patch
This patch introduces self-range (iterators).

And sort function is in:
0005-Repair-misspeled-inplace-of.patch

Iterators is very flexible - not such as in Progress Game, but allows to iterating objects of some kind if there's no target_[object_type] . I must refactor it to made it more flexible.

In Progress iterators allows to select units of player or iterating over players or iterating of each unit of each player, etc. I don't known AI could understand this flexible.

I think, we could made iterating range vector, but it request to add very complicated code.

I think that's time to test everything.

#33 Updated by Sławomir Lach over 2 years ago

Should I format-patch <first-not-my-revision> and send everything it outputs? I give an file name, where missing parts are implemented.

#34 Updated by Sławomir Lach over 2 years ago

I made:
git clone --shared my-freeciv-source new-directory
cd my-freeciv-source
git format-patch -o dir some_hash
cd ../new_directory
<apply each patch by git apply in correct order>
./autogen.sh && make
./fcgui & ./fcser

Everything worked, so prepared archive with dir content.

#35 Updated by Sławomir Lach over 2 years ago

Are you reviewing patches? What's the status?

#36 Updated by Marko Lindqvist over 2 years ago

  • Category set to General
  • Sprint/Milestone set to 3.1.0

Sławomir Lach wrote:

sveinung asked me to integrate changes in Progress related to this topic to Freeciv.

I assume sveinung is the one that will handle this?

#37 Updated by Marko Lindqvist over 2 years ago

Just the number of patches attached (have not looked any of them) suggest that this ticket needs to be split to a lot of tickets. Usually there should be only one feature / ticket.

#38 Updated by Sveinung Kvilhaugsvik over 2 years ago

Sławomir Lach wrote:

Are you reviewing patches? What's the status?

Review started. I sent you a private message explaining why it took so long. I hope to have some feedback tomorrow.

This should, as Marko said, be split in multiple tickets. Do you know what a logical change is? An explanation is given here: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#separate-your-changes I hope to be able to help you split it once I have a better overview.

#39 Updated by Lexxie L about 2 years ago

This would give so much power to ruleset designers. Along with bi-directional requirement checking for the Multiplier/Policy thing. Both of these give such a general power to rise above and be more creative, from inside the ruleset.

#40 Updated by Sławomir Lach about 2 years ago

Ok.
I think I have time beginning at late next week to prepare path. Eventually two weeks later.

#41 Updated by Sławomir Lach about 2 years ago

#42 Updated by Sveinung Kvilhaugsvik about 2 years ago

Please have a look at how the action enabler concept was introduced: https://github.com/freeciv/freeciv/commit/636f7bf13f6fc05071b7a4b17f7c3ddde9b7935e https://github.com/freeciv/freeciv/commit/8de924fa706e51c7cd38dc7307e724053a5f5391

My initial suggestion for breaking this up: Let us start with just enough for me to move Airlift to be a counter. To make it as simple as possible I can hard code the Airlift counter and the interactions with Airlift like checking, setting etc. This means there won't be any need for ruleset code etc in the first patch.

An advantage if I move Airlift to counters early is that you can test via autogames. See https://freeciv.fandom.com/wiki/Testing_Tips

Does this sound like a good plan? Please point out if my initial suggestion could be broken down more but still be in logical changes. You are more familiar with you code than I am.

#43 Updated by Sveinung Kvilhaugsvik about 2 years ago

Sveinung Kvilhaugsvik wrote:

Let us start with just enough for me to move Airlift to be a counter.

I can break it out for you if you don't know how to edit patches in git or if you are uncertain about what would be needed to do this. We can use Feature #876776 for it.

#44 Updated by Sveinung Kvilhaugsvik about 2 years ago

Alexandro Ignatiev wrote:

I doubt the syntax for rulesets is good. The principle that each requirement stays for its own is just good, clear and AI code likely relies on it a lot.

A requirement should defiantly not care about other requirements in its requirements list. (Just a confirmation)

Then, I think even on this early stage we should be ready that counter may change on more than one event

The description mentions changing counters because an action was performed. Must be flexible enough that an effect can be auto-upgraded to it.

#45 Updated by Sveinung Kvilhaugsvik about 2 years ago

Hint: It is good too keep future possibilities in mind when you design something. But use the future possibilities as limitations on the design of the simple feature. Make sure that the simple version don't break the future version by making it impossible to upgrade rules using the simple version to it.

Don't try to implement everything at once. Doing it properly is hard. Focus on getting in one thing at a time. When I sent in the patch introducing action enablers (not even in the ruleset) I already had a design for generalized actions. Generalized actions aren't done yet. That didn't stop me from getting the ready parts accepted around 7 years ago.

#46 Updated by Sveinung Kvilhaugsvik about 2 years ago

Please specify what commit your series is supposed to apply on top of when you send a patch series.

#47 Updated by Sveinung Kvilhaugsvik about 2 years ago

Sveinung Kvilhaugsvik wrote:

Let us start with just enough for me to move Airlift to be a counter.

Patch 1

I'll need:
  • the ability to define a named number (a counter) of the type int with a step of 1 for cities
  • the ability to set the value of a specific counter for a specific city from the C code
  • the ability to get the value of a specific counter for a specific city from the C code

This is the minimum for the first patch. It can be used as a base for additional features like access to counters from Lua, access to counters from effects (or counters_effects if you convince me that those are better) and for moving existing counters like Airlift to your counters framework.

I'll also need:
  • transfer of the value of the counters (of each city) over the network
  • transfer of the counters over the network

Those could be included in the first patch if you want to and they don't make the first patch too large. They could come separate: in one patch or in one patch for each. If you haven't done them at all I can do them my self.

Tolerated as a part of patch 1 as long as the code isn't too long or in need of too much feed back:
  • support for counters for other entity kinds. If it is too long: keep the counter_range enum but with only one member.

Follow up patches

When that is accepted I'll need:
  • the ability to check that a counter is above 0: Feature #859052
  • the ability to add to or subtract from a counter once an action has been successfully performed (Action_Success_Actor_Counter_Add effect in my suggestion in the description)
  • the ability to add to or subtract from a counter on turn change and the ability to reset a counter on turn change. The CityEndTurnCounterAdd effect in the description and a way to reset a counter to 0, say by having counters that never survive turn change, is one way this could be done. The CityEndTurnCounterReset effect from the proposal in the description is another.
  • moving counters the ruleset

Each in their own, separate patch.

#48 Updated by Lexxie L about 2 years ago

Hopefully it's useful comment, and I know he's following design strategy of SIMPLE feature first that is designed in a way that doesn't prevent adding future things to it.

So all I can do is talk about what future things might be wanted...
1. separation of counters - different things get different counts
2. different ways to actively increment/decrement the counter
3. different ways to adjust the limit for the counter, for example, effects or conditions that would change the number of airlifts, etc.
4. ability for it to not be so difficult to make new counters or even user defined counters, later
5. not overspecifying a counter to be stuck tied into only one context. What do I mean? Making sure it's not "tied" to only one thing. For example, do counters have to be tied to a city? How about to other scopes? A nation? A world? e.g., a ruleset might allow a max of one successful "Steal tech" action per turn, a max of one detonate nuke per turn, or who knows, let the ruleset designer figure it out!
6. ability to tie counter to other things via req access, etc. For example, if counter goes over x, then the odds of something else go up (or the bonus or the penalty etc.)

I'm sure people already thought of these things. I know that and I also know it has to start simple first, or it will never get a start. I'm only writing these just in case it's helpful to prevent future limitations.

#49 Updated by Lexxie L about 2 years ago

A philosophic thought occurred to me. What is a counter at its very essence? It's a ruleset var! That's right, it's just giving the ruleset ability to have its own vars.

1. Ability to define it
2. Ability to check greater than, less than, or equal to
3. Ability to increment, decrement, multiply, divide it.

What does it allow, almost infinite flexibility for almost everything. Before, every time someone has a unique new idea, the whole thing has to be put in as special feature. But if a ruleset can keep track of its own "counters" or "vars" with good flexibility for these 3 criteria, all kinds of new power and possibility for rulesets to make their own features, become possible.

Ability to "check" the counter/var, that's just incorporated flexibly into reqs for all the ways to do it.
Ability to increment/decrement etc., that's more tricky. It might come from effects or actions or maybe some other things too?
Ability to define it, this is the part I have no ideas about.

#50 Updated by Alexandro Ignatiev about 2 years ago

Lexxie L wrote:

A philosophic thought occurred to me. What is a counter at its very essence? It's a ruleset var! That's right, it's just giving the ruleset ability to have its own vars.

1. Ability to define it
2. Ability to check greater than, less than, or equal to
3. Ability to increment, decrement, multiply, divide it.

Actually, yes, a counter is a ruleset-defined variable. But we already could make such variables and do whatever to them with Lua. The attempt to make counters is about make it more native way for the game, i.e.
1. more simple to code
2. understandible for agents and AI.
The 2nd point means we are limited in the operations we are going to do to counters. Of course we are going to make it possible to set them with Lua and, probably, a server command, but the general way will be some more rigid operation by game rules.

Sveinung Kvilhaugsvik wrote:

Sveinung Kvilhaugsvik wrote:

Let us start with just enough for me to move Airlift to be a counter.

Patch 1

I'll need:
  • the ability to define a named number (a counter) of the type int with a step of 1 for cities
  • the ability to set the value of a specific counter for a specific city from the C code
  • the ability to get the value of a specific counter for a specific city from the C code

This is the minimum for the first patch. It can be used as a base for additional features like access to counters from Lua, access to counters from effects (or counters_effects if you convince me that those are better) and for moving existing counters like Airlift to your counters framework.

I have written in a sub-task, it's possible to put counter modifiers in effects but it requires to inform the function what counter is modified that may overcomplicate is_req_active() and stuff (and also, some change triggers are actions and some are not, and it's not always clear what participant of an action has a counter modified).

I'll also need:
  • transfer of the value of the counters (of each city) over the network

(note for the future: counter visibility ranges)

Those could be included in the first patch if you want to and they don't make the first patch too large. They could come separate: in one patch or in one patch for each. If you haven't done them at all I can do them my self.

Airlifts are known to the city owner, we have to transfer them, so we must put it into the protocol.

#51 Updated by Sveinung Kvilhaugsvik about 2 years ago

Sveinung Kvilhaugsvik wrote:

Sveinung Kvilhaugsvik wrote:

Let us start with just enough for me to move Airlift to be a counter.

Patch 1

I'll need:
  • the ability to define a named number (a counter) of the type int with a step of 1 for cities
  • the ability to set the value of a specific counter for a specific city from the C code
  • the ability to get the value of a specific counter for a specific city from the C code

This is the minimum for the first patch. It can be used as a base for additional features like access to counters from Lua, access to counters from effects (or counters_effects if you convince me that those are better) and for moving existing counters like Airlift to your counters framework.

I have written in a sub-task, it's possible to put counter modifiers in effects but it requires to inform the function what counter is modified that may overcomplicate is_req_active() and stuff (and also, some change triggers are actions and some are not, and it's not always clear what participant of an action has a counter modified).

I am willing to debate if this feature should be added (I lean towards normal effects rather than special effects) but this does not belong in patch 1.

#52 Updated by Sveinung Kvilhaugsvik about 2 years ago

Sveinung Kvilhaugsvik wrote:

I am willing to debate if this feature should be added (I lean towards normal effects rather than special effects) but this does not belong in patch 1.

English parse failure. I read "I have written in a sub-task," as "I have added this sub task that should go in with patch 1" and not as "I have said in Feature #876776". Sorry about that.

#53 Updated by Sławomir Lach about 2 years ago

Sveinung Kvilhaugsvik wrote:

Please have a look at how the action enabler concept was introduced: https://github.com/freeciv/freeciv/commit/636f7bf13f6fc05071b7a4b17f7c3ddde9b7935e https://github.com/freeciv/freeciv/commit/8de924fa706e51c7cd38dc7307e724053a5f5391

My initial suggestion for breaking this up: Let us start with just enough for me to move Airlift to be a counter. To make it as simple as possible I can hard code the Airlift counter and the interactions with Airlift like checking, setting etc. This means there won't be any need for ruleset code etc in the first patch.

An advantage if I move Airlift to counters early is that you can test via autogames. See https://freeciv.fandom.com/wiki/Testing_Tips

Does this sound like a good plan? Please point out if my initial suggestion could be broken down more but still be in logical changes. You are more familiar with you code than I am.

I think I could split/join my patches, so in result we receive patch describing connected parts, so people could better understood it.

Tomorrow I will probably done the first path, as Sveinung Kvilhaugsvik describes, but without network code. I always have an trouble, when messing up with packets.def, etc.

#54 Updated by Sveinung Kvilhaugsvik about 2 years ago

Sławomir Lach wrote:

I think I could split/join my patches, so in result we receive patch describing connected parts, so people could better understood it.

Great! Could you mention the Freeciv git revision it applies on top of just in case? You can find the git revision by doing git log and scrolling down until you see a revision that came from Freeciv. Example: if you base you patch on Freeciv's master branch right now the git revision is a22dcb2e1a1cfc73bbd698abea576b6da95edf7b but people will usually just write a22dcb2e since the 8 first digits typically is enough to identify a git revision.

Tomorrow I will probably done the first path, as Sveinung Kvilhaugsvik describes, but without network code. I always have an trouble, when messing up with packets.def, etc.

The lack of network code is no problem at all. I'll do it myself as a follow up patch. We can start with counters only being visible to the owner of the entity they are attached to.

#55 Updated by Marko Lindqvist 7 months ago

  • Sprint/Milestone changed from 3.1.0 to 3.2.0

Counters are a 3.2 feature. Haven't checked if this ticket is still relevant.

#56 Updated by Sławomir Lach 7 months ago

About changes. Similar think I introduce in old, C version of Progress. But... If we would save counter name and expected value in requirement name field, we must increase size of some requirement-related union.

About modify counter value by effects, this was main part of my suggestions. But looking around lua signals will make it more flexible.

#57 Updated by Sławomir Lach 7 months ago

Edit:
Your idea makes sense. We can save in requirement universal union short int for counter target related id and in second short int value. Value cannot be greater than 256*256-1 in this case.

#58 Updated by Sławomir Lach 3 months ago

Because somebody asks on forum to let possible implementing religion and convert action (such like in Civ5/6), I propose:
1. Add new action enablers:
counter_change_unit_unit
counter_change_unit_unit_turn_end
counter_change_unit_city
counter_change_unit_city_turn_end
counter_change_unit_tile
counter_change_unit_tile_turn_end

counter_change_city_unit
counter_change_city_unit_turn_end
counter_change_city_city
counter_change_city_city_turn_end
counter_change_city_tile
counter_change_city_tile_turn_end

2. Add OutputCounter reqs
3. When all actor/target requirement passes, we generate name of step counter from outputcounter value and add step counter value to value of outputcounter. Step counter is always contained in actor.
4. Actions with turn_end prefix will be processed after turn end against each join of target and actor.

For example:
ui_name_convert_city_to_islam = "Convert"
[actionenabler_convert_city_to_islam]
action="counter_change_unit_city"
target_reqs = {
"OutputCounter", "IslamFollowers", "City"
}

So we get actor.IslamFollowers_Step and add it to target.IslamFollowers. Unit is an actor and city is a target.

Also available in: Atom PDF