The proposal to merge lp:~widelands-dev/widelands/seafaring-ai into
lp:widelands has been updated.
Status: Needs review = Merged
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
--
Your team Widelands Developers is subscribed to branch
Thanks, Tibor. I will take care of the merging :)
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
___
Mailing list:
SirVer - I added such test:
test_rip_portdock_with_worker_and_ware_in_transit.lua
It is bit lost among other tests, because they all uses the word portdock
when in fact they mean port
Though situation when portdock is lost but cannot be restored and port itself
has to be destroyed is not
Review: Approve
Time to merge then.
The one issue that concerns me still in this branch is that there is no new
test for a warehouse that needs to recreate it's port dock. I feel this corner
case can easily regress without a test. But since this branch has been sitting
for so long, I think
Bug report, so we won't forget:
https://bugs.launchpad.net/widelands/+bug/1421107
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
SirVer - I added such test:
test_rip_portdock_with_worker_and_ware_in_transit.lua
My apologize. I did not remember/see it in the branch. I think this is great
then.
The other test case you describe is indeed harder to do. You would need to
construct a map with military buildings of another
Review: Approve
All tests now run without problems on my machine, so IMO this branch is good to
go now :)
Since the test that's still failing does so only on your machine and for trunk
as well, I expect that there's something about your setup that Widelands
doesn't like.
--
I just completed tests on my debian PC - all passed - I just dont know
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
you can run regression tests again. I will look at this one failing test. Also
I am testing the game with AI-only players, I will let you know if everything
looks alright.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is
not familiar at all... but I will
keep looking at it.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
You are reviewing the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands.
--
https://code.launchpad.net/~widelands-dev/widelands
Yes, trunk runs all the tests successfully (I tested twice and did a fresh
compile on a fresh branch and tested again and it's OK, and it works for SirVer
as well), so the boost problem you're having must be a different problem. Your
test explicitly fails, our test just hangs.
--
I'm still on Boost 1.54
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
___
Mailing list:
Test won't run at all with ./regression_test.py -r
test_rip_first_port_with_worker_in_portdock - I get a file not found. Tibor's
method works though, here is the relevant output:
Trying to run: map:scripting/init.lua: done
Trying to run:
Probably boost is involved to some degree, but I started tracking down what is
triggering the crash.
It seem this situation is it: when a builder is in shipping state, left on a
ship with no ports remaining and following function
GunChleoc - I test mainly the script *SECOND* not FIRST port with portdock
But FIRST... behaves as you described.
You are still sure that trunk behaves differently? Both compiled with the same
environment...
--
I just grabbed myself a fresh trunk and the tests still go through there.
Is there a way to run test_rip_first_port_with_worker_in_portdock.lua without
running the whole testsuite?
./widelands --scenario=test/maps/ship_transportation.wmf doesn't work.
--
Maybe try updating boost? Seems like a boost bug in what you see there.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
./regression_test.py -r test_rip_first_port_with_worker
-r searches for the given regular expression in the test name (combination of
map + lua file).
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
the file is test_rip_second_port_with_worker_in_portdock.lua
After some investigation I found that folowing command will run just particular
script:
./widelands --verbose=true --datadir=. --disable_fx=true --disable_music=true
--language=en_US --scenario=test/maps/ship_transportation.wmf
I am not sure if this is related:
https://github.com/boostorg/signals2/pull/8/files
When i was updating my build environment a few days ago i had to apply this
patch to boost 1.5.7 to make it work with widelands.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Tino,
the patch seems to be aplied...
I can do one test though, insert some printf into warelist.cc and run a test
game to see whether that piece of code is used during running of widelands at
all.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team
with me it does not hang, just prints out FAIL for that test.
I dont have a good network here and have only some older trunk (half year old
about) on harddisk, but tested this trunk and current seafaring and both have
similar behaviour, this is my testing session:
$python2.7
Review: Approve
I have moved the ScoutingDirection enum into ship.h and fixed up the code so it
won't get used as boolean.
Everything else LGTM.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
Review: Needs Fixing
Houston, we have e problem: test_rip_first_port_with_worker_in_portdock.lua
hangs.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
Review: Resubmit
I thought these are brand new tests you just created, nevermind... :)
BUT - I compiled trunk, run the tests and they failed on the very test. So no
regression here...
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands
So I looked at your tests - I liked them much, two comments about the issue:
1.) I think this is not AI related - at all
2.) I would say the lua is not correct, and I am not sure what is to be tested.
When you extend the sleeps - you can see how the ship picks a worker from port1
and if port2
I see the same as SirVer - tests work in trunk.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
___
Mailing list:
BUT - I compiled trunk, run the tests and they failed on the very test. So no
regression here...
not what I see - the test passes for me on trunk and hangs forever in
seafaring-ai. It could be something in the new logic of re-adding the portdock
to a warehouse.
--
Would it be possible to saveload some data? Not objects, only f.e. unordered
sets of integers? It would help much...
Of course you could save state, but it won't help: We support right now that
two humans start playing a game, later one of them loads it and fills the other
slot with an AI.
HEY,
after some painfull debugging I found that on load restored immovables are not
sending notes to players, but instead a player is scanning for them as a part
of late_initialization, see here:
Review: Resubmit
I fixed most of comments.
And I realized that save/load is quite hurting to AI. A lot of internal info of
AI is just lost. Some of them will be restored, but I need to do some
deeper analysis. But not in this branch.
Would it be possible to saveload some data? Not
@SirVer
So I did some simple tests and it is BAD. List of ships is empty after load. So
AI will not manage them at all.
But his is analogous to productionsites, militarysites vectors when loading
game - so I tested militarysites and these are properly initialized = AI knows
about all of them
I found that when a ship is loaded from savefile no note is sent at all. It is
send only when ship is initiated.
with playerimmovable it is called from other place. So I will try to find
better place to sent a note about new ship from...
--
But is seems that there is some mechanism that take care of NoteImmovable
messages and is not in effect for NoteShipMessages.
I do not believe that is correct - if it where late_initialization would not be
needed. Loading would then just be a very fast way of gaining tons of map
objects. I
of course
I tested only AI-only games, because I dont have enough time for a real game...
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
@GunChleoc
first NOCOM is perhaps a mistake??
and second one was discussed with SirVer and I think this is no problem and I
might remove it.
But I will wait for another review and comments
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands
My testing was done to see if anything crashes, and to check if behaviour is
OK. I think it is OK for now - I just decided to give detailed feedback so we
know what can be improved further.
Once the 2 remaining NOCOMS are fixed, I think this branch is ready to be
merged :)
--
I added a NOCOM - there is also a second NOCOM that I don't have an answer for.
I also did some testing, and got no crashes.
- On the Fellowships map, 1/3 AIs managed to start building a shipyard and port
about 30 minutes into the game. The shipyard was correctly stopped until the
port got
so to your comments:
The main problem is that seafaring is based on one-water assumption. There is
no scanning
and analysis of map to identify individual distinct waters and making decision
which one is worth seafaring. This would have to include cheating probably.
I have couple of maps (mine
I think these tests should be done for all players - what you describe can
happen for a human player too if he presses the same button twice while the
command is still distributed to other players in the game. Can easily happen in
slow network games. Your code saveguards against that and should
Correct, your type is the enum class name now.
If you don't give the enum class itself an explicit type, it will default to
int, which would be fine in this case. You also don't need to assign numbers to
the members, this is entirely optional and should only be done when you
explicitly depend
Alright, I fixed what remained. Yet there is one NOCOM with question :)
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
//NOCOM - how is it, when I added class here I have to cast everything
Because enough_ships isn't of the enum class' type. You will get rid of the
cast if you do something like this:
enum class FleetStatus: uint8_t {kNeedShip = 0, kEnoughShips = 1, kDoNothing =
2 };
FleetStatus enough_ships =
Oh, I believed uint8_t is the type. Then 'uint8_t' is not needed at all, is it?
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
Thanks, this makes the diff a lot smaller :)
Give us a shout when you're ready again.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
I cannot see it. What line number?
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is subscribed to branch
lp:~widelands-dev/widelands/seafaring-ai.
___
Mailing list:
Neither can I, seems to be gone or hiding.
It was about changing the enum to an enum class - the change has the advantage
that there is less danger of another enum using the same name and the code
jumping on the wrong enum. We are planning to move from enums to enum classes
in general.
Alright, I merged the trunk once more and resolved conflicts...
There is one NOCOM as a reminder for myself...
I will change that enum (in defaultai.cc), it is defined within a function so
it should not be that dangerous however...
--
but building a port until the
economy has a portdock.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
of portspace is 50 %, 3-4
portspaces (in reasobale spacing) would be enough.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
I think you can maximize the fun by making smaller branches. They are faster to
merge, clean and review.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands
-dev/widelands/seafaring-ai 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
So I implemented comments I saw there, merged trunk, resolved conflicts :(
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev
.
I also saw that there are commented out lines of code in the diff, they should
go before a merge.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai
Running clang-format is good :) Doesn't work on my system though :(
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands
, read the branch info for more
info.
Tested a lot as AI-only games, I would welcome a human players tests and
feedback.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/seafaring-ai into lp:widelands.
=== modified file 'src/ai
Also I run codecheck over couple of files that I had edited, therefore the diff
shows quite a lot of cosmetic changes.
--
https://code.launchpad.net/~widelands-dev/widelands/seafaring-ai/+merge/242271
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev
59 matches
Mail list logo
