[Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
[EMAIL PROTECTED] - Tue Feb 12 22:42:03 2008]:
/**
-...
-**/
-void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best)
-{
- if (best-want cur-want) {
-best = cur;
- }
-}
Just catching up on a week of freeciv mail and noticing this
now...but...surely the above function does nothing? best = cur only
assigns the pointer values, changing nothing in the actual data.
Ha! Nevermind me. Having read the rest of the ticket now I see that is
exactly the bug that caz is fixing.
-jason
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
Fix to actual bug. Needs some more testing to check if this is the
only problem.
- ML
diff -Nurd -X.diff_ignore freeciv/ai/aitools.c freeciv/ai/aitools.c
--- freeciv/ai/aitools.c 2008-01-20 04:02:14.0 +0200
+++ freeciv/ai/aitools.c 2008-02-10 11:28:34.0 +0200
@@ -1168,7 +1168,13 @@
void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best)
{
if (best-want cur-want) {
-best = cur;
+/* This simple minded copy works for now.
+ *
+ * TODO: We should get rid of this function. Choice structures should
+ * be accessed via pointers, and those pointers should be updated
+ * instead of copying whole structures.
+ */
+*best = *cur;
}
}
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
Thank you for finding my mistake in PR#39553!
This worthless function is only called 4 places, and the first place I've
looked is:
ai/advmilitary.c:1223:
if (best_choice.want choice-want) {
/* We want attacker more than what we have selected before */
copy_if_better_choice(best_choice, choice);
That expands to:
if (best_choice.want choice-want) {
/* We want attacker more than what we have selected before */
if (choice-want best_choice-want) {
choice = best_choice;
So, this test (and function) is redundant
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563
Here's my proposed patch to rid us of this troublesome beast
Index: ai/advdomestic.c
===
--- ai/advdomestic.c(revision 14391)
+++ ai/advdomestic.c(working copy)
@@ -336,17 +336,23 @@
init_choice(cur);
/* Consider building caravan-type units to aid wonder construction */
ai_choose_help_wonder(pcity, cur, ai);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
init_choice(cur);
/* Consider city improvements */
ai_advisor_choose_building(pcity, cur);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
init_choice(cur);
/* Consider building caravan-type units for trade route */
ai_choose_trade_route(pcity, cur, ai);
-copy_if_better_choice(cur, choice);
+if (choice-want cur.want) {
+ *choice = cur;
+}
}
if (choice-want = 200) {
Index: ai/aitools.c
===
--- ai/aitools.c(revision 14391)
+++ ai/aitools.c(working copy)
@@ -1163,16 +1163,6 @@
}
/**
-...
-**/
-void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best)
-{
- if (best-want cur-want) {
-best = cur;
- }
-}
-
-/**
...
**/
bool is_unit_choice_type(enum choice_type type)
Index: ai/aitools.h
===
--- ai/aitools.h(revision 14391)
+++ ai/aitools.h(working copy)
@@ -92,7 +92,6 @@
void init_choice(struct ai_choice *choice);
void adjust_choice(int value, struct ai_choice *choice);
-void copy_if_better_choice(struct ai_choice *cur, struct ai_choice *best);
bool is_unit_choice_type(enum choice_type type);
Index: ai/aicity.c
===
--- ai/aicity.c (revision 14391)
+++ ai/aicity.c (working copy)
@@ -1365,7 +1365,9 @@
!(ai_on_war_footing(pplayer) pcity-ai.choice.want 0
pcity-id != ai-wonder_city)) {
domestic_advisor_choose_build(pplayer, pcity, newchoice);
- copy_if_better_choice(newchoice, (pcity-ai.choice));
+ if (pcity-ai.choice.want newchoice.want) {
+pcity-ai.choice = newchoice;
+ }
}
}
Index: ai/advmilitary.c
===
--- ai/advmilitary.c(revision 14391)
+++ ai/advmilitary.c(working copy)
@@ -1218,9 +1218,10 @@
best_choice, ferryboat, boattype);
}
- if (best_choice.want choice-want) {
+ if (choice-want best_choice.want) {
/* We want attacker more than what we have selected before */
-copy_if_better_choice(best_choice, choice);
+*choice = best_choice;
+
CITY_LOG(LOG_DEBUG, pcity, kill_something_with()
%s has chosen attacker, %s, want=%d,
city_name(pcity),
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 On 10/02/2008, William Allen Simpson wrote: Here's my proposed patch to rid us of this troublesome beast I'd rather keep the function. Even though 'better' choice is currently determined simply by comparing want value, that's not necessarily true in the future. Currently logic of setting the want value is ugly and in some cases just broken (magic numbers, cap values prohibiting big want of one kind to interfere with want value of other kind...) We should clean up all that mess and put the logic to just one central point: copy_if_better_choice(). - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 OK, seems reasonable. I'd say commit. ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 Autogames produce only tons of: 1: Maria Theresia's Graz(54,37) [s2 d0 u0 g0] Falling back - didn't want to build soldiers, workers, caravans, settlers, or buildings! with actual player/city varying, of course. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
URL: http://bugs.freeciv.org/Ticket/Display.html?id=39563 On 14/08/07, Marko Lindqvist [EMAIL PROTECTED] wrote: Autogames produce only tons of: 1: Maria Theresia's Graz(54,37) [s2 d0 u0 g0] Falling back - didn't want to build soldiers, workers, caravans, settlers, or buildings! with actual player/city varying, of course. At the same time, more and more different crashes show up. Does William have a fix, or should we revert (PR#39525, r13286)? Don't know how difficult it would be to remove just changes brought by r13286 and changes committed to fix its problems. I'm not yet ready to revert all development since. - ML ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
