[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-22 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Hopefully committed correctly to S2_2 (r14846) and trunk (r14847).

There were some files added/moved and lots of changes to
makefiles, so be sure to re-run autogen.sh.


--
ほとんどできなかった。

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-20 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

2008/6/19 Madeline Book:

 When I get the OK, I'll commit this editor work so far
 so that I can continue implementing other editor features.

 Looks good to me.

 Meanwhile, I am having a problem with my method of composi-
 ting sprites to make icons (functions create_terrain_pixbuf,
 create_military_base_pixbuf and create_tile_pixbuf). For
 some reason the sprite.offset_{x,y} fields seem to mean
 something different for terrains than they do for the
 military base sprites. For example, some base sprites have
 negative offsets!

 Sorry, can't help you there. When I changed tilespec code to handle
military bases a bit differently from other specials, offset stuff was
just recreated as it was before. Are unit sprite offsets handled same
way as terrain sprite offsets, or do they use (yet) another system?


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-13 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [book - Wed Jun 11 20:43:45 2008]:
 
  [EMAIL PROTECTED] - Wed Jun 11 19:15:25 2008]:
 
   - packhand.c:2368: handle_tile_info: Assertion
  `unit_list_size(ptile-units) == 0' failed. I were removing vision
  from tile with somebody else's unit.
 
 This looks like a bug due to a condition I failed to forsee.
 It should be easy enough to fix (i.e. don't allow removing
 vision of a player on a tile with that players' units/city
 in it). I'll get right on fixing it.

This actually turned out to be caused by something
different than what I expected. It seems the client
expects tiles changing seen state from known seen
to have no units on it:

client/packhand.c +2357
  if (TILE_KNOWN_SEEN == old_known  TILE_KNOWN_SEEN != new_known) {
/* This is an error.  So first we log the error, then make
   assertion. But for NDEBUG clients we fix the error. */ 
unit_list_iterate(ptile-units, punit) {   
  freelog(LOG_ERROR, %p %d %s at (%d,%d) %s, 
  punit,   
  punit-id,   
  unit_rule_name(punit),   
  TILE_XY(punit-tile),
  player_name(unit_owner(punit))); 
} unit_list_iterate_end;   
assert(unit_list_size(ptile-units) == 0); 
unit_list_unlink_all(ptile-units);
  }

So my idea is to send a packet_unit_remove to the
client for every unit on the tile if none of the units
give vision to the client's player (i.e. neither owned
by the client's player nor by any player that really
shares vision with the player). Is there anything
wrong with this approach?

Or would it be better to just have the client remove
the units itself in the above quoted section?


--
早く返事して下さいね

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-12 Thread Jason Dorje Short

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Madeline Book wrote:

 I actually like the whole 3 line ... header as a visual
 separator between functions (thanks in large part due to the
 nice color difference in syntax hilighting). Anyway, that is
 just my preference; are you suggesting that empty headers
 should just be:
 
 /
 /
 
 Or perhaps no header at all? 

On that I'm not sure.  I actually never found a function that couldn't 
make use of a 1-line descriptive comment yet.  I'd probably say the 
empty header should just have the comments with no line in between though.

-jason



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-12 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [book - Wed Jun 11 19:56:11 2008]:
 
  [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]:
 
   - There's some new direct S_FORTRESS and S_AIRBASE references.
  That goes against work for generic military bases. See common/
  base.[ch] to see if better implementation is already possible.
 
 That I did not know about. I'll look into using that code.
 
 Most of the time I had problems trying to get a generic sprite
 for various objects (like roads or fortresses) so the added
 code in client/tilespec.c might not be the best possible approach.

Alright after examining the new generic military-base code I
have reached some conclusions regarding the use of S_FORTRESS
and S_AIRBASE in the editor code (off-topic rant about the
approach in general posted in PR#37356, which you can feel free
to ignore if you've heard before or just have not had time to
address the issues raised there).

Please correct me if I am wrong but the references to 
S_FORTRESS and S_AIRBASE that are being objected to are in
the function get_basic_special_sprite in client/tilespec.c.

My argument is that as long as S_FORTRESS and S_AIRBASE are
a part of enum tile_special_type this reference is simply
unavoidable. Furthermore if I were to say add some manner
of conversion function (e.g. base_by_special, special_is_base)
I would have to check for this and add annoying special-case
code every time I iterate over the special types (which is
very common considering the requirements of the editor).

In any case, changes will have to be made in the future
when military-base and special handling changes; I'll be
sure to update the editor code as that is finalized.

I will however look into using the {fore,middle,back}ground
sprites more intelligently (I suppose they should be combined
into one sprite to actually make the full representative
sprite of an airbase or fortress).

--
誰があの余計なことに興味を持っているか

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-12 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Thanks for the thorough reply. My ideas for specific issues
follow below.

 [EMAIL PROTECTED] - Thu Jun 12 21:21:14 2008]:
 
  Please don't assume that bases are stored as specials. Iterating
 specials should already skip S_FORTRESS and S_AIRBASE (at least it did
 when I last worked on this)

The macro tile_special_type_iterate doesn't skip over them, so I
suppose I will make the change and post that as a new ticket.

  I realize that editor code may require some new accessor functions. I
 can write those in a proper manner, if you tell me what you need. Note
 that tile.c has functions for checking, adding and removing bases from
 tile.

I think the existing accessors are sufficient. If I am missing
something I'll add it myself or ask for ideas if I am unsure
of the implementation.

  I would have to check for this and add annoying special-case
  code every time I iterate over the special types (which is
  very common considering the requirements of the editor).
 
  Isn't common iterator macro no longer doing this?

As I said above, tile_special_type_iterate does not, but I
will fix it so that it does.

Then in the other parts of the editor code, after iterating
specials, I'll add code that iterates over bases (using
base_type_iterate).
 
  I will however look into using the {fore,middle,back}ground
  sprites more intelligently (I suppose they should be combined
  into one sprite to actually make the full representative
  sprite of an airbase or fortress).
 
  No, those separate parts are for a reason. Depending on tileset,
 units in fortress may appear in front of fortress background, but
 behind foreground. There certainly is a reason for middleground too,
 even though I cannot remember it now.

I assumed that drawing the foreground over the middleground over
the background would make an acceptable icon. ;)

But I'll check this some more.


--
今おいしいラーメンを食べる

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-11 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]:
 
  I only read sources, not yet tested.
 
 
  -  Please don't add new empty function headers. New code should have
 function headers already when committed.

What do you mean by empty function header? If you mean the

/*
  ...
**/

Then I only fill that in if the function name is not descriptive
enough or there are some special conditions or side-effects that
programmers need to be aware of (e.g. you must free the return
value yourself).

Do you think I should describe the functions more in the header?

  - You add function client_player(), but don't always use it in new
 code instead of client.conn.playing

This I will fix. It is because I added the client_* functions
after already writing some code using client.conn.playing and
forgot to update the old code. :(
 
 
  What really happens to these files:
 
 --- a/client/gui-gtk-2.0/Makefile.am
 +++ b/client/gui-gtk-2.0/Makefile.am
 @@ -42,8 +42,10 @@ libguiclient_a_SOURCES = \
 dialogs.h   \
 diplodlg.c  \
 diplodlg.h  \
 -   editdlg.c   \
 -   editdlg.h   \
 +  editgui.c   \
 +  editgui.h   \
 +  editprop.c  \
 +  editprop.h  \
 
  Are editdlg.[ch] renamed, split or completely replaced?

They are completely replaced. Similarly for client/include
/editgui_g.h. The point was that the code in editgui.[ch]
is more that just an edit dialog so the name editdlg did
not really apply anymore (cf. function editgui_tileset_changed
and editgui_refresh which affect more than one widget).

I added editprop.[ch] because editgui.[ch] was getting very
long. It will contain the properties dialog (see the wikia
editor page for an overview) and its helper functions.

  - There's some new direct S_FORTRESS and S_AIRBASE references. That
 goes against work for generic military bases. See common/base.[ch] to
 see if better implementation is already possible.

That I did not know about. I'll look into using that code.

Most of the time I had problems trying to get a generic sprite
for various objects (like roads or fortresses) so the added
code in client/tilespec.c might not be the best possible approach.

  - Add FIXME about the fact that following code is not generic enough
 (gen-movement)
 +  coastal = is_sailing_unittype(punittype);
 +  homecity = find_closest_owned_city(pplayer, ptile, coastal, NULL);

This is a direct conversion of the old can_unit_exist_at_tile
to work on unit_type's (needed for example to check if a unit
type can be created at a certain tile).

I'll add the FIXME here, but generalizing this code really
belongs in another ticket.
 
  - Why city creation now fails without telling reason to client?
 (modifications to handle_edit_city_create() /
 handle_edit_create_city() )

I thought a long time about how and when to do error messages
in server edithand.c. I decided the server should for the most
part remain silent for bad input or illegal operations. This
is because the user (who is editing) is assumed to know the
rules (e.g. where cities may be placed) and becase operations
can be batched (which would result in a lot of duplicate
messages).

Do you think the server should notify the user more in
such circumstances?

  This is huge patch. Let's make only agreed fixes against this patch
 before commit (It's impossible for others to track what other changes
 appear if you do other development).

It's not hard for me to stash local changes and variously merge
between branches, but I will address now the issues you have raised
before extending the functionality further.


--
何も欲しくない人がこの世界のどこかにいるか

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-11 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Wed Jun 11 19:15:25 2008]:
 
  After some testing:
 
  - Only functionality missing from this new model, but present in old
 version seems to be modifying city internals

I don't recall there being any GUI for doing that and the
packets, handlers, etc., were variously #if 0'd out along
with some FIXMEs. Anyway the property editor (in editprop.[ch])
that I would be working on now and will be working on soon
will be able to do that (also for tiles, units, and possibly
players).

  - packhand.c:2368: handle_tile_info: Assertion
 `unit_list_size(ptile-units) == 0' failed. I were removing vision
 from tile with somebody else's unit.

This looks like a bug due to a condition I failed to forsee.
It should be easy enough to fix (i.e. don't allow removing
vision of a player on a tile with that players' units/city
in it). I'll get right on fixing it.


--
皆さん頑張ろうね!

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-11 Thread Jason Dorje Short

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Madeline Book wrote:

  - Why city creation now fails without telling reason to client?
 (modifications to handle_edit_city_create() /
 handle_edit_create_city() )
 
 I thought a long time about how and when to do error messages
 in server edithand.c. I decided the server should for the most
 part remain silent for bad input or illegal operations. This
 is because the user (who is editing) is assumed to know the
 rules (e.g. where cities may be placed) and becase operations
 can be batched (which would result in a lot of duplicate
 messages).
 
 Do you think the server should notify the user more in
 such circumstances?

This is an issue for a lot of regular-game city report operations as 
well.  There is no established standard afaik (there should be), but 
most places seem to have client checking as well as server spamming of 
error messages IIRC.

-jason



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-11 Thread Jason Dorje Short

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Madeline Book wrote:
 URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 
 
 [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]:

  I only read sources, not yet tested.


  -  Please don't add new empty function headers. New code should have
 function headers already when committed.
 
 What do you mean by empty function header? If you mean the
 
 /*
   ...
 **/
 
 Then I only fill that in if the function name is not descriptive
 enough or there are some special conditions or side-effects that
 programmers need to be aware of (e.g. you must free the return
 value yourself).
 
 Do you think I should describe the functions more in the header?

Previous discussions have agreed that empty comments are not helpful. 
It's possible that the function name really says everything about the 
function; in such a case maybe ... isn't needed.  But I really don't see 
any use to having ... as a comment.

-jason



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-11 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Thu Jun 12 04:43:02 2008]:
 
 Madeline Book wrote:
  URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 
  
  [EMAIL PROTECTED] - Wed Jun 11 16:51:03 2008]:
 
   I only read sources, not yet tested.
 
 
   -  Please don't add new empty function headers. New code should 
have
  function headers already when committed.
  
  What do you mean by empty function header? If you mean the
  
  /*
...
  **/
  
  Then I only fill that in if the function name is not descriptive
  enough or there are some special conditions or side-effects that
  programmers need to be aware of (e.g. you must free the return
  value yourself).
  
  Do you think I should describe the functions more in the header?
 
 Previous discussions have agreed that empty comments are not helpful. 
 It's possible that the function name really says everything about the 
 function; in such a case maybe ... isn't needed.  But I really
 don't see any use to having ... as a comment.

I actually like the whole 3 line ... header as a visual
separator between functions (thanks in large part due to the
nice color difference in syntax hilighting). Anyway, that is
just my preference; are you suggesting that empty headers
should just be:

/
/

Or perhaps no header at all? 


-
勘弁してくれ。

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-10 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Tue Jun 10 12:37:35 2008]:
 
 2008/6/9 Madeline Book
  With your blessing I will being to merge this new
  editor gui code into S2_2 and trunk. That way afterwards
  individual commits can stay relatively small and examinable.
 
  Currently I concentrate to 2.1.5 release, so I'm not looking in to
 your patch just yet. I'm not asking you to postpone commit if doing so
 will make further development harder for you (you have waited the
 required 24h for comments already, and there's no point in waiting
 forever just because others have no time) but if you can keep on
 developing even when this is not committed, I would appreciate
 postponing this to next week.

No problem, it doesn't inconvenience me at all. I'll just keep
working on my local repository and post diff snapshots every week
or two (and perhaps expand the internal summary on the wikia page).


--
喜んで。問題ないよ

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-06-10 Thread Jason Dorje Short

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

On Tue, Jun 10, 2008 at 11:32 AM, Madeline Book [EMAIL PROTECTED] wrote:

 URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Tue Jun 10 12:37:35 2008]:

 2008/6/9 Madeline Book
  With your blessing I will being to merge this new
  editor gui code into S2_2 and trunk. That way afterwards
  individual commits can stay relatively small and examinable.

  Currently I concentrate to 2.1.5 release, so I'm not looking in to
 your patch just yet. I'm not asking you to postpone commit if doing so
 will make further development harder for you (you have waited the
 required 24h for comments already, and there's no point in waiting
 forever just because others have no time) but if you can keep on
 developing even when this is not committed, I would appreciate
 postponing this to next week.

 No problem, it doesn't inconvenience me at all. I'll just keep
 working on my local repository and post diff snapshots every week
 or two (and perhaps expand the internal summary on the wikia page).

Personally I'd rather it be committed (once caz gets time to look at
it) so if I get some time to take a look I can go straight into fixing
things.

-jason



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-28 Thread Ulrik Sverdrup

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

2008/4/28 Madeline Book [EMAIL PROTECTED]:

  URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

  The usual status update, gentlemen.

  I will have more time to work on this this week since
  the stuff I wanted to do for longturn is done now. ;)


You've made the Global observer work again, I noticed! Would it work
in a networked game too? I haven't looked into the details in the
changes. If so, could we break out that part and commit it as soon as
possible? AFAIK Marko would be happy to have the global observer back.
And I would too.

ulrik



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-28 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [engla - Mon Apr 28 16:15:54 2008]:
 
 2008/4/28 Madeline Book [EMAIL PROTECTED]:
 
   URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 
 
   The usual status update, gentlemen.
 
   I will have more time to work on this this week since
   the stuff I wanted to do for longturn is done now. ;)
 
 
 You've made the Global observer work again, I noticed! Would it work
 in a networked game too? I haven't looked into the details in the
 changes. If so, could we break out that part and commit it as soon as
 possible? AFAIK Marko would be happy to have the global observer back.
 And I would too.

See PR#40139. Since I needed global observer to work for testing
editor features, I had need to apply the patch posted there to
my local editor branch. Similarly with PR#40124.


--
料理が全然上手じゃないという噂を聞いた。


___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-06 Thread Marko Lindqvist

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

On 04/04/2008, Madeline Book  wrote:

  A small preview of the editor toolbar in action, as
  described in the editor wikia page.
  I have also attached a screenshot if you do not have
  the time (or the means) to apply a git binary diff
  and compile the result (if you do, don't forget to
  run autogen.sh).

 Care to send editor.png separately so I can take just the code
changes from patch, and not to struggle with binary part? Looks like
good development.


 - ML



___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


[Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-06 Thread Madeline Book

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

 [EMAIL PROTECTED] - Sun Apr 06 16:20:31 2008]:
 
 On 04/04/2008, Madeline Book  wrote:
 
   A small preview of the editor toolbar in action, as
   described in the editor wikia page.
   I have also attached a screenshot if you do not have
   the time (or the means) to apply a git binary diff
   and compile the result (if you do, don't forget to
   run autogen.sh).
 
  Care to send editor.png separately so I can take just the code
 changes from patch, and not to struggle with binary part?

Can do.


--
そして地獄に迷い込んだ女と会うこと。
inline: editor.png___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-05 Thread Per Inge Mathisen
On Fri, Apr 4, 2008 at 10:45 AM, Daniel Markstedt [EMAIL PROTECTED] wrote:

  URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

  Very nice, Madeleine! I'm thinking of starting a forum thread to
  attract the attention of Freeciv's creative minds. :)

Yes, really awesome. I am happy to see that someone is working on the
editor again.

  - Per

___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev


Re: [Freeciv-Dev] (PR#40184) [editor] Toolbar GUI front-end preliminary work

2008-04-04 Thread Daniel Markstedt

URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

Very nice, Madeleine! I'm thinking of starting a forum thread to
attract the attention of Freeciv's creative minds. :)

On 4/4/08, Madeline Book [EMAIL PROTECTED] wrote:

  URL: http://bugs.freeciv.org/Ticket/Display.html?id=40184 

  A small preview of the editor toolbar in action, as
  described in the editor wikia page.

  Internals are still mostly stubs, which I am now
  working on. ;)

  I have also attached a screenshot if you do not have
  the time (or the means) to apply a git binary diff
  and compile the result (if you do, don't forget to
  run autogen.sh).

  I would welcome perhaps better icons, if any of the
  freeciv artists would care to make some; the ones I
  threw together are mostly just cut-and-pasted from
  random places and could probably be improved.


  --
  じゃー、それは?いい星ですね。どのぐらい?

 ___
  Freeciv-dev mailing list
  Freeciv-dev@gna.org
  https://mail.gna.org/listinfo/freeciv-dev






___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev