Crash on rendering roads

Bug #1638394 reported by Notabilis
14
This bug affects 2 people
Affects Status Importance Assigned to Milestone
widelands
Fix Released
Medium
Unassigned

Bug Description

I am not completely sure what causes it, but my theory is:
- Discover/See an area but do not conquer it.
- Lose sight of the area.
- Let the AI conquer and build on the land without observing it. Building is important here, we need some roads.
- At some later time, rediscover the ares. Roads leading into the non-visible areas crash the game. (Most likely these roads must lead into darkness above or left of the visible area).

Happened in r8163. The problem seems to be that the road is told to be rendered by a neighboring tile but the start of the road is not visible yet. Since it is not visible, the player(!=game) does not know the owner of the land, leading to a failed assert(owner!=null).

A backtrace and a branch with a fix are attached. I can also offer savegames if required.

Tags: crash
Revision history for this message
Notabilis (notabilis27) wrote :
Revision history for this message
GunChleoc (gunchleoc) wrote :

Fix LGTM. Candidate for Build 19?

Changed in widelands:
milestone: none → build19-rc2
tags: added: crash
Revision history for this message
SirVer (sirver) wrote : Re: [Bug 1638394] Re: Crash on rendering roads

Very likely this was introduced with the zoom branch. Of course not sure.

> Am 01.11.2016 um 22:37 schrieb GunChleoc <email address hidden>:
>
> Fix LGTM. Candidate for Build 19?
>
> ** Changed in: widelands
> Milestone: None => build19-rc2
>
> ** Tags added: crash
>
> --
> You received this bug notification because you are subscribed to
> widelands.
> https://bugs.launchpad.net/bugs/1638394
>
> Title:
> Crash on rendering roads
>
> Status in widelands:
> New
>
> Bug description:
> I am not completely sure what causes it, but my theory is:
> - Discover/See an area but do not conquer it.
> - Lose sight of the area.
> - Let the AI conquer and build on the land without observing it. Building is important here, we need some roads.
> - At some later time, rediscover the ares. Roads leading into the non-visible areas crash the game. (Most likely these roads must lead into darkness above or left of the visible area).
>
> Happened in r8163. The problem seems to be that the road is told to be
> rendered by a neighboring tile but the start of the road is not
> visible yet. Since it is not visible, the player(!=game) does not know
> the owner of the land, leading to a failed assert(owner!=null).
>
> A backtrace and a branch with a fix are attached. I can also offer
> savegames if required.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/widelands/+bug/1638394/+subscriptions

Revision history for this message
GunChleoc (gunchleoc) wrote :

You completely reworked the road program when you kicked out software rendering - might it have happened there?

@Notabilis: can you reproduce this on b19-rc1?

Revision history for this message
Notabilis (notabilis27) wrote :

I can not reproduce this on b19-rc1.
Of course, I am not completely sure what is triggering it, but if my theory is correct b19 seems to be fine.
When my theory is correct, it should be a relatively frequent case when exploring with / using ships, so the bug might be quite new.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for testing - reassigned to Build 20.

Changed in widelands:
milestone: build19-rc2 → build20-rc1
Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

I think I just suffered from this bug. I created a bug report(1639570) after discovering this one. It looks like its the same problem. See attached my savegame to reproduce the bug.

Happend in r8169.

Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :
Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

The fix does not work with my savegame. It still crashes.

Revision history for this message
Notabilis (notabilis27) wrote :

I can confirm your savegame crashing in both trunk and the "fixed" branch.

I haven't looked into the source yet but it seems like the fix handles only existing roads the player discovers (my crash case) while in your case a new building is created with a road (building to flag) leaving the visible area.

Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

I pushed a branch with a fix which works for my savegame but after thinking a bit about the whloe thing, I am not sure at all if this is realy a solution in every case.

The owner of the starting field is only needed to fetch the right texture of the road. The field where the road begins might not be visible to the player. In my solution I suspect that the field where the road ends is always visible. But I do not realy think that this proposition holds.

The branch is here:
lp:~janosch-peters/widelands/bug-1638394-render-road

The main problem with this bug is, that we want to know the real owner of the field and not the owner from the players point of view. One way to fix this would be to introduce a field real_owner in FieldsToDraw. Another option would be to pass a parameter in add_road which contains the "real" field or the real field owner of the field.

Revision history for this message
Notabilis (notabilis27) wrote :

I don't think it will work the way you fixed it. I haven't tried, but it probably will crash when you can only see the upper end of the road (e.g. enemy to the lower right of your land).

But I like the idea. I only did a short try, but what if we create a new local "owner" variable which is set to either start.owner or end.owner, whichever is not null? At least one of them should be not null and contain the right owner, since the method is only called when the street is at least partially visible (i.e. at least one end). In a short test (loading our crashing save games) it seemed to work.

I have slight concerns about what will happen if the player can see one end of the road and the other endpoint ends in an area which once was owned by another enemy. I could imagine that the road is drawn for the wrong tribe but I haven't tried it yet. (Node that this effect should also be present in trunk.)

Revision history for this message
GunChleoc (gunchleoc) wrote :

I have added Janosch to widelands-dev, so you can work on the branch together if you want.

@Janosch: There is a "Change Branch Details" link on the top right where you can change the branch owner to widelands-dev.

Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

@Notabilis: I implemented your version. Could you please upload your savegame so that I can run some tests (and tell me at what time the crash happens in your savegame)?

Revision history for this message
Notabilis (notabilis27) wrote :

Thanks, no problem. It is a custom map but I think everything should be included.
The crash happens after a few seconds as soon as the soldiers enter the tower.

Just as a remark: It is even possible to create savegames "containing" the crash. As long as one does not look at it, the game runs fine.

Revision history for this message
xxx-deleted (janosch-peters-deactivatedaccount) wrote :

Ok, I tested your savegame and the fix works also in this situation. I comitted the fix on this branch:

lp:~widelands-dev/widelands/bug-1638394-render-road

Code check and regression tests are OK. The change now needs a reviewer.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks for the fix - reviewed :D

Would you guys like to review some of my branches, so I can get them in? You already know a lot more C++ than I did when I started doing code reviews here. To do a review, have a look at the code to see if anything rubs you the wrong way about it, and do a quick test to check if it works as expected. Travis will take care of regression tests etc. And if you miss a bug, it's not the end of the world :)

Changed in widelands:
status: New → Fix Committed
Changed in widelands:
importance: Undecided → Medium
assignee: nobody → Janosch Peters (janosch-peters)
Revision history for this message
GunChleoc (gunchleoc) wrote :

Fixed in build20-rc1

Changed in widelands:
assignee: Janosch Peters (janosch-peters) → nobody
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Duplicates of this bug

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.