patchFreeciv - Patches: patch #3776, ...

 
 
Show feedback again

patch #3776: server/barbarian.c:create_barbarian_player fix deficencies

Submitted by:  Not Given <imhotep>
Submitted on:  Mon 11 Mar 2013 07:51:58 PM UTC  
 
Category: NonePriority: 5 - Normal
Status: NonePrivacy: Public
Assigned to: NoneOpen/Closed: Open
Planned Release: 

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Thu 14 Mar 2013 12:14:40 AM UTC, comment #3:

Sorry, I just had a bloody encounter with the local police.
(Bloody on my side, violence one-directional.)

Don't know when I'll be able to contribute.

Not Given <imhotep>
Tue 12 Mar 2013 08:57:00 PM UTC, comment #2:

comment #1

It may be that this patch is ahead of its time.

I don't see any code preventing more than one barbarian/pirate nation (may be only because I did not look at the right places).

I agree on the point that there currently is /popper randomness/ in choosing one out of only one possible barbarian nations per type ;-)

But as soon as this limitation is lifted, it fails, possibly with segmentation fault.

Not Given <imhotep>
Tue 12 Mar 2013 06:53:27 PM UTC, comment #1:

> If there is more than one of them, the first found by iterating
> the list is returned. Which one that is might be implementation
> dependent but in all cases lacks proper randomness.


You mean existing barbarian players? That's correct as currently there can be only one barbarian nation of the type, so first one is the only possible choice.
Initial creation of the barbarian player does have proper randomness amoung possible nations, I think.

> to cite previous posts or properly include links


gna adds some links automatically, such as "bug #number", "patch #number" "comment #number" when you simply mention those.

Marko Lindqvist <cazfi>
Project Administrator
Mon 11 Mar 2013 07:51:58 PM UTC, original submission:

This patch is for 2.4.99-dev r22446

Aside from the fact that the name is misleading (it does not normally create a new barbarian),, the function server/barbarian.c:create_barbarian_player lacks randomness and prefers recycling of already existing barbarians before creating new ones.

Existing code:
If there is a barbarian player of the requested type, it is reused.
If there is more than one of them, the first found by iterating the list is returned. Which one that is might be implementation dependent but in all cases lacks proper randomness.

Only if no barbarian of the requested type is found, a new one is created.

If for some reason the pick_a_nation-function returns NULL, the server crashes with segmentation fault.

Patch (intended behavior, probably needs review and more testing):
One of the existing barbarians with requested type is chosen truly random from the existing ones.
If an new barbarian player of that type can be created, it is chosen with a probability equal to the already existing ones.

If there is no available nation for the new barbarian, it is not considered.

Reason for the patch:
the patch removes some hidden bugs (the code where the server crashes is not reached in the original code only because no new barbarians are created once one of each type is present).

In view of http://gna.org/bugs/?17606 there will be more than one barbarian nation for each type, making the random choose more complex than the present "choose one out of one".

In view of the new gameloss_style options ( https://gna.org/bugs/?20577 ), when there is only one barbarian this one and only gets far too strong.

Other issues:
In the preexisting code, when an existing barbarian player is reused, it gets 100 gold extra. I don't know if this is really intended, but as I don't know otherwise, I ported this feature to the new code.

Have a lot of fun.

BTW: I have not found out yet how to see a preview of a submit nor a description on how to do some common features, so as to cite previous posts or properly include links. So, until someone cares to give me a hint on any errors, you all will have to live with my self-made style.

Not Given <imhotep>

 

(Note: upload size limit is set to 1024 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by cazfi (Posted a comment)
  • -unavailable- added by imhotep (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follows 1 latest change.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 11 Mar 2013 07:51:58 PM UTCimhotepAttached File-=>Added create_barbarian_player_22446.patch, #17419
    Show feedback again

    Back to the top


    Powered by Savane 3.1-cleanup