Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/add_custom_building into lp:widelands
Yes, I'd liker to hear your suggestions :) Thanks for testing again! @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
Review: Approve Did a complete test of the empire 4 scenario ans some other test regarding the trainingsites_max_percent value. Also had a look to the code. The branch now handles the value in a way that it will not crash anymore. The scenario ran through without any observations. So I would say the branch is ready for merging. However I am not very happy with the algorithm handling the value and the definition of these values. But i will open a new bug for this. -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
@Gun,I somehow believed that this is part of AI hints - that way there would be no problem with same building across multiple tribes... I just want to emphasize that any sum > 100% is fine, no big harm would be done. Only if sum < 100% can cause problem. -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
ok I'll change my branch after this one has gone into trunk then. What zip file (which content)do you want me to upload? -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
@hessenfarmer: Let's keep your scenario separate, because it hasn't been reviewed yet and the strings would land on Transifex, causing extra work for the translators. It would be great if you could upload a zip to a bug though, then I could use it to help add a test to out test suite. @TiborB: I am now thinking that we should shift that calculation to the AI and do it per player - theoretically, 2 tribes could have the same building, messing up the calculations. Let's do that in another branch though - maybe we can have a common branch where we shift both this and the persistent data to the AI directory. -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
I hope it's fixed now, could you please test again with the values that caused the exception? -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
If you allow less then 100%, the result will be that AI will build one trainingsite of each type and never more.. this is mathematics... Further trainingsits are built only if current ratio of this type vs all trainingsites is < this percentage -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
Review: Needs Fixing Ok tested it, found a bug in postload_calculate_trainingsites_proportions(): in the data only the value for an empire_arena is defined with 20%. With my additional custom trainingcamp there were 3 remaining sites (colosseum, Trainincamp and custom trainingcamp). They got 26% each which summed up to only 98 which threw an exception as beeing less than 100%. did a workaround by assigning another 20% to my custom Trainingcamp which led to 30% each for the remaining 2 and so we got the correct sum. in this way the game loaded fine and ran as designed so the basic mechanism should be fine. But the branch still contains the savegame bug of r8493 so I need more time to playtest the whole scenario as I can't save. Anyhow I think we should fix the calculation of of the trainingpercentages by allowing less than 100%. By the way what is the use of this AI Hint value? never noticed it before. -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/add_custom_building. ___ 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/add_custom_building into lp:widelands
Ok I'd like to propose the following: 1. will download the branch 2. will download the related appveyor build 3. will integrate the last verion of emp04 scenario. 4. test it and report back 5. if succesful we just need to merge this branch to have everything ready. would this be ok -- https://code.launchpad.net/~widelands-dev/widelands/add_custom_building/+merge/334062 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/add_custom_building 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
