Donation

Help us to pay our server!
(: Consider a donation :)



Social Media

  • Facebook
  • Google+

Latest Posts

Topic: main roads becoming normal again

ypopezios
Avatar
Joined: 2018-04-20, 00:22
Posts: 135
Ranking
At home in WL-forums
Posted at: 2018-05-16, 17:12

einstein13 wrote:

Maybe I will do a contribution one day for that and it will be solved from my perspective face-grin.png .

You can always solve it locally for your computer ;P

einstein13 wrote:

But maybe not another forum. Rather another topic.

Of course another topic, but it has to belong in some subforum. "Technical Help" doesn't sound appropriate for it.

einstein13 wrote:

If we want to make a simple patch - simple one topic. But if you are going to rebuild whole system (like me), we will need proper way of communication.

The idea is to rebuild the whole system, but one patch at a time. I'm open to proper ways of communication.


Top Quote
GunChleoc
Avatar
Joined: 2013-10-07, 15:56
Posts: 2398
Ranking
One Elder of Players
Location: RenderedRect
Posted at: 2018-05-17, 07:50

ypopezios wrote:

GunChleoc wrote:

Maybe we can add some code to the carriers' "idle" task to fix this?

ETA: found it

If there is an "idle" task, your code could work. But I have the impression that idle carriers become fully "taskless" on line 111:

It works with this really simple fix, so there is no need to add any complicated code instead. The only time this is not called if there is no ware transport happening in the whole economy at all, in which case we don't care anyway. I have tested it.

Swapping the ware routing is a good project face-smile.png

ypopezios wrote:

ETA: What would be the right subforum to start such a thread in?

Game Suggestions fits best I think.

Edited: 2018-05-17, 07:53
Top Quote
ypopezios
Avatar
Joined: 2018-04-20, 00:22
Posts: 135
Ranking
At home in WL-forums
Posted at: 2018-05-17, 13:39

GunChleoc wrote:

It works with this really simple fix, so there is no need to add any complicated code instead. The only time this is not called if there is no ware transport happening in the whole economy at all, in which case we don't care anyway. I have tested it.

I wish it was that simple. Actually I already tried an almost identical fix (the commented-out charge) and it didn't work. I tested your version and it doesn't work either. Fully-idle roads never get demoted, they fall into endless sleep, even if attached flags are full of traffic.

Moreover, the debug version doesn't compile anymore. There are many ways to fix this, but they depend on your coding style.


Top Quote
ypopezios
Avatar
Joined: 2018-04-20, 00:22
Posts: 135
Ranking
At home in WL-forums
Posted at: 2018-05-17, 17:03

GunChleoc wrote:

ypopezios wrote:

The LAST_DIRECTION stuff sounds like a poor choice, but I'm not sure. What I know is what I want to do, and that is to simply iterate over the 6 roads of the flag (i.e. "for each road in roads").

6 means "iterate over 6 directions". LAST_DIRECTION means "iterate over all directions" - this makes the code safer, and also instantly tells whoever reads the code why it is 6.

i < 6 is used many times in that file. Consider changing all of them for consistency. But in read-only cases (lines 76, 177, 246, 280, 520, 528, 725) this is way better:

for (Road* const road : roads_)
Edited: 2018-05-18, 03:29
Top Quote
GunChleoc
Avatar
Joined: 2013-10-07, 15:56
Posts: 2398
Ranking
One Elder of Players
Location: RenderedRect
Posted at: 2018-05-22, 07:13

ypopezios wrote:

GunChleoc wrote:

It works with this really simple fix, so there is no need to add any complicated code instead. The only time this is not called if there is no ware transport happening in the whole economy at all, in which case we don't care anyway. I have tested it.

I wish it was that simple. Actually I already tried an almost identical fix (the commented-out charge) and it didn't work. I tested your version and it doesn't work either. Fully-idle roads never get demoted, they fall into endless sleep, even if attached flags are full of traffic.

According to the log output, my new line for demoting idle roads is working:

NOCOM DEMOTE idle road
NOCOM charge wallet: 0, carriers: 1, gamesecs: 20778721, last_charge: 20776891, steps: 2 NOCOM new wallet: -1
NOCOM PAY for road: 0, carriers: 1, queue: 0, steps: 3
NOCOM charge wallet: 12, carriers: 1, gamesecs: 20779019, last_charge: 20717244, steps: 3 NOCOM new wallet: -49
NOCOM DEMOTE idle road
NOCOM charge wallet: 0, carriers: 1, gamesecs: 20784283, last_charge: 20779019, steps: 3 NOCOM new wallet: -5
NOCOM PAY for road: 0, carriers: 1, queue: 0, steps: 2
NOCOM charge wallet: 8, carriers: 1, gamesecs: 20786123, last_charge: 20724528, steps: 2 NOCOM new wallet: -53
NOCOM DEMOTE idle road
NOCOM charge wallet: 0, carriers: 1, gamesecs: 20787959, last_charge: 20784283, steps: 3 NOCOM new wallet: -3
NOCOM DEMOTE idle road
NOCOM charge wallet: 8, carriers: 1, gamesecs: 20793413, last_charge: 20786123, steps: 2 NOCOM new wallet: 1
NOCOM DEMOTE idle road
NOCOM charge wallet: 1, carriers: 1, gamesecs: 20795243, last_charge: 20793413, steps: 2 NOCOM new wallet: 0

The problem is that the empty wallet doesn't trigger a demotion, so that is where the fix needs to be.

Moreover, the debug version doesn't compile anymore. There are many ways to fix this, but they depend on your coding style.

One of the website binaries isn't linking - I have no idea why yet.

I'll be separated from my development machine over the summer, so I won't be able to look into this any time soon. I have to concentrate on bugfixing for Build 20.

Just use the release builds for testing in the meantime.

ypopezios wrote:

GunChleoc wrote:

ypopezios wrote:

The LAST_DIRECTION stuff sounds like a poor choice, but I'm not sure. What I know is what I want to do, and that is to simply iterate over the 6 roads of the flag (i.e. "for each road in roads").

6 means "iterate over 6 directions". LAST_DIRECTION means "iterate over all directions" - this makes the code safer, and also instantly tells whoever reads the code why it is 6.

i < 6 is used many times in that file. Consider changing all of them for consistency. But in read-only cases (lines 76, 177, 246, 280, 520, 528, 725) this is way better: ~~~~ for (Road* const road : roads_) ~~~~

Agreed, but not in this branch. The bigger a branch, the harder the review, and it has nothing to do with road promotions. There are many cleanups like this that need doing, but we don't have enough people to do them.


Top Quote
ypopezios
Avatar
Joined: 2018-04-20, 00:22
Posts: 135
Ranking
At home in WL-forums
Posted at: 2018-05-22, 13:55

GunChleoc wrote:

According to the log output, my new line for demoting idle roads is working:

[log]

The problem is that the empty wallet doesn't trigger a demotion, so that is where the fix needs to be.

The log confirms that your line is executed for some roads (my line did too). But those roads are only temporarily idle (and most of them are not promoted either, so the "DEMOTE idle road" message is misleading). Fully idle roads never reach that line, because no code is executed for them during their sleep.

GunChleoc wrote:

One of the website binaries isn't linking - I have no idea why yet.

The debug version doesn't compile because it complains for your implementation of constant kMaxWallet. The release version escapes that because it optimizes constants. It's all explained here.

GunChleoc wrote:

Just use the release builds for testing in the meantime.

Release builds don't give me any info, so testing with them is like in blind-mode. If you don't plan to address that soon, better revert to function-level constants, for debug version to be available.

GunChleoc wrote:

There are many cleanups like this that need doing, but we don't have enough people to do them.

Is there a place where those cleanups are listed for someone to address them in the future? Or is the mess so big that only a general cleanup is planned (and postponed for ever)?


Top Quote
Tibor
Joined: 2009-03-23, 23:24
Posts: 1166
Ranking
One Elder of Players
Location: Slovakia
Posted at: 2018-05-22, 14:16

I think if you was able to compile the game by yourselves, a lot of issues would be gone: kMaxWallet issue, you could have any debug output you wish...

Also there is no sense to have a list of needed cleanups as there is no pool of developers aching for such list...


Top Quote
GunChleoc
Avatar
Joined: 2013-10-07, 15:56
Posts: 2398
Ranking
One Elder of Players
Location: RenderedRect
Posted at: 2018-05-23, 07:49

We do have a list of wanted cleanups: https://bugs.launchpad.net/widelands/+bugs?field.tag=cleanups

Feel free to open new bugs and add the tag face-smile.png

ypopezios wrote:

GunChleoc wrote:

One of the website binaries isn't linking - I have no idea why yet.

The debug version doesn't compile because it complains for your implementation of constant kMaxWallet. The release version escapes that because it optimizes constants. It's all explained here.

I know. I can read the logs. The problem isn't what, it's why, and I won't have any time for tracking this down for at least 2 months.

I have a bunch of already finished features that won't make Build 20 because we're short handed on code review. So, I won't delay the release any further for an unfinished feature - this feature will be just as great in Build 21. Setting up a build environment in Windows does take time, but simply having bzr explorer so that you can pull and push branches would take you about 15 minutes. Then, you could try fixing this yourself - it's probably just a missing include somewhere


Top Quote
WorldSavior
Avatar
Joined: 2016-10-15, 04:10
Posts: 603
Ranking
One Elder of Players
Location: Saxony, GER
Posted at: 2018-05-23, 17:48

ypopezios wrote:

WorldSavior wrote:

Would you have fun at improving the Artificial Intelligence? I could imagine that algorithms could play a very important role there.

That's a well-advised suggestion (although I would expect Tibor to make it first). Still teaching the AI to deal with the shortcomings of Widelands is less productive than freeing Widelands from those shortcomings.

I would be very interested if you could give some examples. I don't see how you could free Widelands from such shortcomings without changing the game too much.

I hope that the official Widelands will be always as complex as it is now (approximately), so in this case one has to improve the AI instead of changing the game, if one wants to get a better AI.

Therefore, the logical first step would be to improve Widelands' models, creating a more AI-friendly environment in the process.

How?

Maybe it would make a lot of sense to teach the AI a clever road system for maps with perfect building ground, like in this description here: https://wl.widelands.org/forum/topic/2834/?page=1#post-19815


*insert phrase about the necessity of saving the world here*

Top Quote
hessenfarmer
Avatar
Joined: 2014-12-11, 23:16
Posts: 280
Ranking
Tribe Member
Location: Bavaria
Posted at: 2018-05-23, 21:08

WorldSavior wrote:

I hope that the official Widelands will be always as complex as it is now (approximately), so in this case one has to improve the AI instead of changing the game, if one wants to get a better AI.

I fully aggree to this. However the Ai thing is complicated. I think Tibors effort to implement a generic algorithm solution is good for tuning the values in the algorithms. But the algorithms and the models need to be working efficiently as well. This is leading to two possible fields of work to improve the AI. Problem might be that after an algorithm change the training is set back to a state worse than already achieved, but some weaknesses in algorithms only appear after Ai has been trained sufficiently. Short story having a look in the AI algorithms might be a good idea to identify any weaknesses.

Therefore, the logical first step would be to improve Widelands' models, creating a more AI-friendly environment in the process.

How?

Maybe it would make a lot of sense to teach the AI a clever road system for maps with perfect building ground, like in this description here: https://wl.widelands.org/forum/topic/2834/?page=1#post-19815

Problem is that an algorithm of road building should be working for all maps. The suggestion in the discussion as far as I understand is to first build a road system then build the buildings. Currently AI is evaluating the need to build a building and afterwards tries to link all buildings by roads. So the change would rather be big and can't be tested easily. For the beginning a clever algorithm to find the shortest road to the closest flag could be an improvement. Perhaps another algorithm to identify and solve bottlenecks could be an improvement as well.
Funny thing is that the discussion contains a lot of stuff regarding balancing of tribes that I face again in my attempt to improve frisians balancing. Any help on this issue would be appreciated as well. face-wink.png

Edited: 2018-05-23, 21:08
Top Quote