[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/geologist-messages into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Agreed, looking good :D @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Continuous integration builds have changed state: Travis build 4014. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/431698863. Appveyor build 3811. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_geologist_messages-3811. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Review: Approve Wow now it looks good in my eyes. And it is clearly visible while still being a good overall design. Thanks a lot for another feature to have more graphical diversity. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
OK, brightened the flag a bit, made the cross slightly darker, and shifted the hotspot for all fri resis as you suggested. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Continuous integration builds have changed state: Travis build 4012. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/431547762. Appveyor build 3809. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_geologist_messages-3809. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
now I tested the new "none" resi in the game. It is really getting better now. I have experimented with the hotspot as with the current hotspot the flags of the building help are directly in front of the flagpole. with a hotspot of (0,46) the resi flag is placed on the right side of the build help flag and is much better visible. This could be a setting for all frisian resi in my eyes. Regarding the little flag of the "none" resi: it looks very good and fits well to the overall frisian resi concept. To make it even better it should be a little brighter cause it has the same brightness as the grey mountains. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
>From a first look into the .png this should be fine will test it in the game >this weekend. @GunChleoc: Which graphics do you mean. I think most of them (if not all) have been discussed in the original branch and the 2 bugs that arose with them will be fixed with this branch. Anyhow if you tell me what to look at I will do so over the weekend. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
OK, you have convinced me with the message icons. I'll have to look at the other graphics sometime next week, unless hessenfarmer greenlights them first. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Okay, next try with a very small flag (animated) with a cross. The flag is barely visible, just enough not to vanish behind the buildhelp, so it can´t be confused with anything; the flagpole is as before. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Well this would be better visible but imho it doesn't fit to a flagpole / flag concept and doesn't look that good either. So if a bigger knob does not look good as well we have two options: Either you give it a try with having a small (smaller than the normal flags) flag with an x at half mast to increase visibility while hopefully not causing confusion with stones found or you leave it like it is and accepting that the empty poles are not outstanding from the yellow buildhelp icons. (with buildhelp off they are clearly visible) Only other thing that comes to my mind that might be worth a try is to move the indicator to the left so they won't be hidden behind the yellow buildhelp flags. In every case thanks a lot for your effort and patience with my comments -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
A bigger knob wouldn´t look good imho. At least it doesn´t in Blender, and it´s hard to guess whether something looks good in-game if it looks bad while designing it. I have instead crossed the flagpole now, how about this? -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Review: Approve test I tested the message box finally and I think the usage of the editor images in the message title fits very well. The Res Indicator is used in the message body and is looking good there as well so I think we should have this branch. If possible It would even be better to have the knob of the flagpoles bigger to better see the empty flagpole for the "none" Resi. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
@ Nordfriese: to test the message box I need to wait for anppveyor or build myself. Will report back later. Although I believe that the swap of the images will be very good for visibility and readability og the message box, because the non quadratic images will ever be porrly visible in the message title lines. @ GunChleoc: I think this would fit well although we show the editor image in the title. Cause the image there si scaled so small, all that really matters is color. and the editor images provide that much better than the ressource indicators. In the body the Indicators fit better cause they are tribe specific. Just quickly excahged the none image and a coal image. None visibility is better now but with buildhelp on it is still not ideal. If we won't go for a flag with a cross we should at least make the pole a little bit thicker and especially the knob of the flagpole bigger. For the ressources I would have preferred two equal flags (like the top one is) but that is a matter of taste. the same for the height of the poles. for my taste they could be even shorter (especially when having two equal flags but that is not that important. So your choice. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
The editor icons were already being used in the message body… -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
I still do not like having the editor resource images as message icons. I'd prefer to keep it as it is, or have the resource indicator for both. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
@hessenfarmer: Uploaded improved graphics for the frisian resis to this branch, how about these? -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
@hessenfarmer: I think the bug is caused by that that the old resis were squarish while some of the new ones are much taller than wide, so the title is not indented as far as for other messages @GunChleoc: I now put the resi image in the message body, so the two images are now swapped compared to trunk -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
According to how it looks in the message box it might be that the bug is hidden in the format rather than the image I would suspect the hardcoded padding value. https://bazaar.launchpad.net/~widelands-dev/widelands/trunk/view/head:/src/logic/map_objects/tribes/worker.cc#L972 -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Review: Needs Fixing I am against this change - it replaces a symbol that the player knows with a symbol that is never, ever seen on the map except in the editor. How about trying it the other way around, to see how that looks? -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/geologist-messages. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/geologist-messages into lp:widelands has been updated. Commit message changed to: Geologist messages use the resource's image as icon instead of the resource indicator's For more details, see: https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/geologist-messages into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Continuous integration builds have changed state: Travis build 3997. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/430196840. Appveyor build 3794. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_geologist_messages-3794. -- https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/geologist-messages into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/geologist-messages into lp:widelands
Benedikt Straub has proposed merging lp:~widelands-dev/widelands/geologist-messages into lp:widelands. Commit message: Geologist messages use the resource´s image as icon instead of the resource indicator´s Requested reviews: Widelands Developers (widelands-dev) Related bugs: Bug #1793021 in widelands: "ressource indicators wrong display in message box" https://bugs.launchpad.net/widelands/+bug/1793021 For more details, see: https://code.launchpad.net/~widelands-dev/widelands/geologist-messages/+merge/355267 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/geologist-messages into lp:widelands. === modified file 'src/logic/map_objects/tribes/worker.cc' --- src/logic/map_objects/tribes/worker.cc 2018-09-10 12:32:56 + +++ src/logic/map_objects/tribes/worker.cc 2018-09-18 18:26:02 + @@ -959,7 +959,7 @@ const ResourceDescription* const rdescr = world.get_resource(position.field->get_resources()); const TribeDescr& t = owner().tribe(); - const Immovable& ri = game.create_immovable( + game.create_immovable( position, t.get_resource_indicator( rdescr, (rdescr && rdescr->detectable()) ? position.field->get_resources_amount() : 0), @@ -980,7 +980,7 @@ get_owner()->add_message_with_timeout( game, std::unique_ptr( new Message(Message::Type::kGeologists, game.get_gametime(), - rdescr->descname(), ri.descr().representative_image_filename(), + rdescr->descname(), rdescr->representative_image(), rdescr->descname(), message, position, serial_, rdescr->name())), rdescr->timeout_ms(), rdescr->timeout_radius()); } ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
