Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1825932-open-games into lp:widelands
Having a message in the chat window isn't enough. I did not see it, and the same will happen to others. So, I still think that we should show the games as unconnectable rather than hiding them. Other things we could to is make the text in the editbox red and adding a tooltip to the editbox. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1827182-sort-client-list into lp:widelands
Review: Approve LGTM :) This branch can go in once codecheck has been fixed. If you don't want to trigger a compile for this, you can use cmake/codecheck/CodeCheck.py src/* | grep -v "src/third_party" -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/bug-1827182-sort-client-list into lp:widelands
Done. :) -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/build20 into lp:widelands
Continuous integration builds have changed state: Travis build 4856. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/527411124. Appveyor build 4637. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_build20-4637. -- https://code.launchpad.net/~widelands-dev/widelands/build20/+merge/366850 Your team Widelands Developers is requested to review the proposed merge of lp:widelands/build20 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1825932-open-games into lp:widelands
I agree. We should change the colour in the editbox and have a tooltip. But currently I have no idea how to change the text's colour. Can you give me a hint, please? -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/expedition_portspace_indicator into lp:widelands
Review: Resubmit OK, now InteractiveBase receives a notification instead when a portspace is found. Since there are fairly many reasons why a port space can become unavailable again (ship sails on, port is constructed, ship is sunk…), the IBase takes care of cleaning up the set itself now when think()ing. This fixes the bug that the indicator won´t go away on portconstruction. Also added saving support for the indicators so they are also shown for ships that already are in PortSpaceFound state when the game is loaded. -- https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/tribe-economy-settings into lp:widelands
Review: Disapprove short review I go along with Benedikt. An average player will have a _lot_ of trouble to get the economy going and will have no clue what the problems are. We would have to rethink all the Tutorials and Scenarios. Some novice players may be trapped in on of the tribes special pitfalls even faster then now (e.g. trying to build a silk infrastructure without silk, winery without Marble etc...) If you want to create a challenge for experienced players, lets have a startcondition like "Limited Start Resources" or such -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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/tribe-economy-settings into lp:widelands
Continuous integration builds have changed state: Travis build 4861. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/527614965. Appveyor build 4642. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_tribe_economy_settings-4642. -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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/representative_image_in_font_renderer into lp:widelands
Review: Approve Code LGTM. Tested briefly, no issues seen. -- https://code.launchpad.net/~widelands-dev/widelands/representative_image_in_font_renderer/+merge/363781 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/representative_image_in_font_renderer. ___ 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/refactor_gameclient into lp:widelands
* Used parent instead of This. * moved all Variables into Impl About that -2 magic: I got lost in finding the sematics of user/player etc. I added a TODO comment with our best guess by now. I am at the End of my vacation (sigh) so lets get this in before it starts lingering around for too long. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands
Found a first bug, still need some debugging -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands
Still get a heap-use-after-free related to std::unique_ptr loader_ui(new UI::ProgressWindow()); But this is maybe just a problem as the game crashed with disconnect(CLIENT_CRASHED, ) A have a deja vue around GameClientImpl.modal which is released twice. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands
You could also do the game tips like this:
const std::string tribename = get_players_tribe();
assert(Widelands::tribe_exists(tribename));
std::unique_ptr tips(new GameTips(*loader, {"general_game",
"multiplayer", tribename}));
As to the ASan crash, I suspect that there is a problem with the lifetime of an
object caused by the pulling out of stuff into separate functions.
--
https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/refactor_gameclient 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1825932-open-games into lp:widelands
Tooltip is implemented. But the changing text colour... that's beyond my understanding. Thought I could implement this with boost::format but that doesn't work. We have set_color() available for tables and textfields, but I have no idea how to implement this for editboxes. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/expedition_portspace_indicator into lp:widelands
I am still wondering if we can get more performance out of this thing. * Ship sails on and ship is sunk could be taken care of by the ShipNote * Port being built can be taken care of by buildcaps, just like we do for the building spaces overlays in general Any other reasons? +1 for the saving support :) -- https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/bug-1825932-open-games into lp:widelands
I actually don't know whether changing text color is supported at this time. I don't want to touch the editbox code right now, because I have a big refactoring in the works. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1827182-sort-client-list into lp:widelands
@bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/bug-1825932-open-games into lp:widelands
Sadly it doesn't. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/expedition_portspace_indicator into lp:widelands
> Port being built can be taken care of by buildcaps IMHO we should remove the portspaces either only in response to Notes or only autonomously. Mixing both feels messy. To use notes, we´d need subscriptions (in addition to what we already have) to – NoteExpeditionCanceled – FieldPossession (another possible reason why a portspace might become unavailable is that some other player conquers that land, which is not even necessarily reflected in the buildcaps) – FieldTerrainChanged (if the port space is removed by a terrain-changing script) – A tree grows over the port space (or is this forbidden somehow?) → is there a notification for that? – A script unsets a port space directly → is there a notification for that? That´s quite a lot for a pretty small feature IMHO; and I don´t think the autonomous cleanup of displayed portspaces is performance-critical – I can´t imagine a player will have so many expeditions at a time that it really matters. -- https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/tribe-economy-settings into lp:widelands
I guess what we could have here is to allow players to define their own default options, because micromanaging this to the same values for each game start is probably a bit annoying. -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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/bug-1825932-open-games into lp:widelands
I have created a merge request for the other branch: https://code.launchpad.net/~widelands-dev/widelands/font_size-lua/+merge/366938 Once that is in, I can take care of the color. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1825932-open-games/+merge/366860 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/bug-1825932-open-games 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
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/tribe-economy-settings into lp:widelands
I agree with Benedikt and Klaus as well as I think some of the defaults could be reduced (e.g. basic weapon from 30 to 10). I believe setting the values to the default starting values would be the lowest limit to reduce to as it would trigger production immediately if something is used. However having individual settings would be the best solution so +1 for this from my side. As a side effect this would lead to individual settings possibilities for the AI which might be good as well -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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/ferry into lp:widelands
Continuous integration builds have changed state: Travis build 4865. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/527627946. Appveyor build 4646. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_ferry-4646. -- https://code.launchpad.net/~widelands-dev/widelands/ferry/+merge/351880 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/ferry. ___ 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/font_size-lua into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/font_size-lua into lp:widelands has been updated. Commit message changed to: Except for scenario and win condition messages, define all font styles via Lua. Reduce the number of construtors in TextArea. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/font_size-lua/+merge/366938 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/font_size-lua 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/bug-1827182-sort-client-list into lp:widelands
Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways. Travis build 4866. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/527644741. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/bug-1827182-sort-client-list into lp:widelands
Continuous integration builds have changed state: Travis build 4866. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/527644741. Appveyor build 4647. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_bug_1827182_sort_client_list-4647. -- https://code.launchpad.net/~widelands-dev/widelands/bug-1827182-sort-client-list/+merge/366843 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1827182-sort-client-list. ___ 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/expedition_portspace_indicator into lp:widelands
Review: Approve -- https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/expedition_portspace_indicator into lp:widelands
You have convinced me. I have done more testing - all working now :) @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/refactor_gameclient into lp:widelands
Continuous integration builds have changed state: Travis build 4871. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/527824464. Appveyor build 4652. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_refactor_gameclient-4652. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient 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/expedition_portspace_indicator into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/expedition_portspace_indicator into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/expedition_portspace_indicator/+merge/366640 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/expedition_portspace_indicator. ___ 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/tribe-economy-settings into lp:widelands
Bug filed: https://bugs.launchpad.net/widelands/+bug/1827696 Shall we discuss the rest on the forum? -- https://code.launchpad.net/~widelands-dev/widelands/tribe-economy-settings/+merge/366878 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/tribe-economy-settings. ___ 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
