Menu
▾
▴
[pio-trackers] [ pio-Patches-3172063 ] Impovement in the editor: toolbar
|
From: SourceForge.net <no...@so...> - 2013-02-10 12:23:11
|
Patches item #3172063, was opened at 2011-02-03 14:20 Message generated for change (Comment added) made by rclobus You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305095&aid=3172063&group_id=5095 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: None Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Micah Bunting (amnykon) >Assigned to: Nobody/Anonymous (nobody) Summary: Impovement in the editor: toolbar Initial Comment: added terrain, port, and chit toolbar. changed resize. added resize for top and right side of map. resize buttons focus_on_click=false. clear hexes on resize. removes ports on adjacent hexes when hex clears when required. swapped col and row variable names in build_map_resize. fixed infinite loop when loading a map >MAP_SIZE. added pictures to describe hex, node, and edge arrangement. added nav definitions and changed functions to use them removed calc_adjacent(hex->map, hex->x, hex->y, adjacent); from connect_network. this is my first patch, please tell me what you think ---------------------------------------------------------------------- >Comment By: Roland Clobus (rclobus) Date: 2013-02-10 04:23 Message: Finally, I've reviewed this patch... The review was done on svn revision 1669, as that was the time this patch was written. I like the toolbar, it really can make the design process of a new board layout easier. I'm uncertain how the toolbar should behave... As noted earlier, I think a discussion on pio...@li... will make it easier to discuss the feature. A follow-up mail will be sent... ---> Read on the mailing list for more ---------------------------------------------------------------------- Comment By: Roland Clobus (rclobus) Date: 2012-11-10 08:45 Message: I could not find your reply on 2011-07-17 in my e-mail archive, so I missed it... I'll review your updated patch soon. ---------------------------------------------------------------------- Comment By: Micah Bunting (amnykon) Date: 2012-11-07 07:40 Message: Just a quick question, What is plan with this patch? The editor toolbar is easier and faster to use then the context menu. I also want to update the hex group patch to work with this after it is committed in the repository. ---------------------------------------------------------------------- Comment By: Micah Bunting (amnykon) Date: 2011-07-17 19:21 Message: I updated toolbar patch with the following changes: - gint terrain to Terrain terrain. - removed sensitive changes to chit_buttons when a new terrain is selected, since selecting a new chit when a non chit terrain is selected, deselects the terrain. - selecting port deselects chit. - selecting a chit deselects port. - redraw chits when selecting a port or chit while a non supporting terrain is selected. - added documentation to added functions in editor.c. - changes global current_hex to local hex. - removes select_port_resource_cb, select_port_direction_cb, select_terrain_cb required only for the context menus. I like the background images for the ports and chits. My idea was to have the background transparent, but fell short with a white background which clashes with the grey toolbar, especially when selected. Regarding the remarks: - When placing a port, the port is facing the closest edge to the click location, if a port can legally be placed there. if not it chooses an allowable direction. - Tiles are modified by clicking on a hex, depending on what is selected on the toolbar. If only the terrain is selected, only the terrain is changed. If only the chit is selected, only the chit is changed. If only the port, only the port is changed. - I do not believe the context menu is required. The context menu allows changing terrain, chit, port, port direction, and a random option. The toolbar allows for all except for the random option. I would like to replace the current random option with the random hex groups, which will provide more options when randomizing maps, as well as making the maps easier to edit. - I do not know what you mean by the inselected terrain type or the active hex, but I think you are referring to the global variable current hex. The variable was from the old context menu and changed to local variables. ---------------------------------------------------------------------- Comment By: Roland Clobus (rclobus) Date: 2011-06-13 05:40 Message: I've updated the patch. The resize functionality is already in svn, the toolbar is still in this patch. My changes: - I've use cairo where needed to match the current code - I've placed the image of the active terrain under the chits, in order to have the correct colours in the toolbuttons Remarks: - I seem unable to set the port direction - It's not immediate apparent how I could modify a tile, perhaps a context menu is still needed for a tile. - The state with inselected terrain type is not clear to me. - The code introduces the concept of 'active hex', but it isn't made visible. I think some worked-out specs (on the pio-devel mailing list) is needed before this patch can be applied. ---------------------------------------------------------------------- Comment By: Micah Bunting (amnykon) Date: 2011-02-10 00:55 Message: I have addressed all but one of the comments and two bugs that were in it. The #define operations are replaced with functions. In my opinion the use of the functions makes the code easier to read and understand then using the arrays used in build_network and connect_network. The expose functions in editor.c and legend.c have been moved into theme.c and theme.h. This adds a possibility of a future patch that displays the theme before selecting it. I could not figure out how to use 'make reindent' I saw some info on it, but not enough to fully understand it. Can someone send me more info on it so I can use it in the future? ---------------------------------------------------------------------- Comment By: Roland Clobus (rclobus) Date: 2011-02-05 01:38 Message: Hello Micah, Thanks for your patch. The end result is very appealing. It is much easier to create maps. Regarding the code I have some remarks: - The code didn't build initially, gdk_pixmap_unref is deprecated, use g_object_unref instead - Could you reformat the code with 'make reindent'? You introduce another coding style - In common/map.[ch] you introduce a global variable, and many #defines to operate on it. Is it really necessary to introduce a global variable? Can the #define operations be replaced by regular functions? Are all these changes required just to introduce hex_free and move_hex? I'm in favor of applying the minimal required change. - Why does guimap_find_edge require a static variable? And why is its result const? - The expose functions in editor.c should not be copied, but be moved to some file in common/gtk - You changed the order of change_width and change_height in in editor.c, which complicates the patch file. - Could you use C-style comments instead of C++? - For clearer naming: could you rename RESIZE_[START|END]_[ADD|SUB] to RESIZE_[INSERT|REMOVE]_[TOP|BOTTOM|LEFT|RIGHT] ? - Regarding functionality: why isn't a terrain type and a dice number active per default? I see no need to maintain the old context menus when the toolbar is present. The amount of comments is quite high, but I really like the added functionality. Regarding the inclusion in the Subversion repository, I would suggest that you take another look at the patch. I'll be able to look at new modifications or be able to apply the requested modifications no earlier than Saturday, Februari 12. With kind regards, Roland Clobus ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=305095&aid=3172063&group_id=5095 |
