[Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
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
http://bugs.freeciv.org/Ticket/Display.html?id=39563 >
/**
-...
-**/
-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.
-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
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
Re: [Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
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
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
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
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
http://bugs.freeciv.org/Ticket/Display.html?id=39563 > On Feb 4, 2008 2:28 PM, William Allen Simpson <[EMAIL PROTECTED]> wrote: > As Jason notes, the problem is that Per set the log level to LOG_ERROR, > but the comments say this is OK Indeed. In the absence of anyone to fix it, changing it to LOG_DEBUG might at least let us read the actual error messages. -jason ___ 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
http://bugs.freeciv.org/Ticket/Display.html?id=39563 >
Found the old report and merged with it. Added some PR# links, instead
of the revision numbers Marko used in the report.
Funny, I've been waiting for Marko to fix this, as it's his original
report, but now I see that Per flagged it for me! Since Per never
sent me an email, I'd no idea And I've been too busy fixing the
myriad 2.1 crashers to worry about non-crashing log messages like this!
As Jason notes, the problem is that Per set the log level to LOG_ERROR,
but the comments say this is OK
ai/aicity.c: ai_city_choose_build()
/* Fallbacks */
if (pcity->ai.choice.want == 0) {
/* Fallbacks do happen with techlevel 0, which is now default. -- Per */
CITY_LOG(LOG_ERROR, pcity, "Falling back - didn't want to build
soldiers,"
" workers, caravans, settlers, or buildings!");
pcity->ai.choice.want = 1;
However, I'll note that this was repoted after I calloc'd the city
(instead of malloc). Before I touched it, nobody ever cleared the
ai.choice, so want (and boat) had garbage in them.
(I discovered the ai.founder_boat problem working on PR#39365 in May,
that led to finding more and more uninitialized ai variables.)
If you look closely, I've even documented the former lack of
initialization in common/city.c: create_city_virtual()
/* pcity->ai.choice; placeholder for searching */
pcity->ai.founder_boat = FALSE;
All in all, though, I'm not really the person to go hunting for the
ai.choice.want problem. Per would be the expert maintainer, and
Marko was the original reporter. All I've done is initialize it.
___
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
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. > Unlike I said in earlier mail, this particular change in AI is caused by some of the revisions r13297 - r13303. r13296 works, but r13303 (next trunk version that compiles) does not. When fix for this has been found, we should run regression test between r13296 and r13303 + fix. Autogames between r13285 and r13286 do differ also. This should be investigated. - 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
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
[Freeciv-Dev] (PR#39563) [Bug] AI doesn't want to build anything in trunk / default.ruleset
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
