Units Demo triggers Assertion failed

[Langbart]

GIT version: [26742]

to reproduce

• Start the Units demo map

EDIT (6/May/22) The best way to reproduce the bug is to test the units_demo before [26775] was committed. This way it is easier to trigger the bug.

GAME STARTED, ALL INIT COMPLETE
Assertion failed: "it->second.pos.X.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.width() && it->second.pos.Y.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.height()"
Location: CCmpUnitMotion_System.cpp:435 (Move)

Call stack:

(error while dumping stack: Function not supported)
errno = 22 (Invalid alignment)
OS error = ?


(C)ontinue, (S)uppress, (B)reak, Launch (D)ebugger, or (E)xit?

Process 97509 stopped
* thread #1, name = 'main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x000000010109c930 pyrogenesis`::mozalloc_abort(msg=<unavailable>) at mozalloc_abort.cpp:33:3 [opt]
   30  	#ifdef MOZ_WIDGET_ANDROID
   31  	  abortThroughJava(msg);
   32  	#endif
-> 33  	  MOZ_CRASH();
   34  	}
   35  	
   36  	#ifdef MOZ_WIDGET_ANDROID

lldb

(lldb) bt
* thread #1, name = 'main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
  * frame #0: 0x000000010109c930 pyrogenesis`::mozalloc_abort(msg=<unavailable>) at mozalloc_abort.cpp:33:3 [opt]
    frame #1: 0x0000000101b945ad pyrogenesis`::abort() at mozalloc_abort.cpp:82:3 [opt]
    frame #2: 0x00000001004f31a6 pyrogenesis`sys_display_error(text=<unavailable>, flags=6) at unix.cpp:268:4 [opt]
    frame #3: 0x00000001004d5923 pyrogenesis`debug_DisplayError(wchar_t const*, unsigned long, void*, wchar_t const*, wchar_t const*, int, char const*, long volatile*) [inlined] CallDisplayError(text=L"Assertion failed: \"it->second.pos.X.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.width() && it->second.pos.Y.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.height()\"\r\nLocation: CCmpUnitMotion_System.cpp:435 (Move)\r\n\r\nCall stack:\r\n\r\n(error while dumping stack: Function not supported)\r\nerrno = 22 (Invalid alignment)\r\nOS error = ?\r\n", flags=6) at debug.cpp:374:8 [opt]
    frame #4: 0x00000001004d5906 pyrogenesis`debug_DisplayError(description=L"Assertion failed: \"it->second.pos.X.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.width() && it->second.pos.Y.ToInt_RoundToZero() / PUSHING_GRID_SIZE < m_MovingUnits.height()\"", flags=6, context=0x00007ffeefbfd840, lastFuncToSkip=<no summary available>, pathname=<no summary available>, line=<no summary available>, func=<no value available>, suppress=<no summary available>) at debug.cpp:460 [opt]
    frame #5: 0x00000001004d618e pyrogenesis`debug_OnAssertionFailure(expr=<no summary available>, suppress=<no summary available>, file=<no summary available>, line=<no summary available>, func=<no value available>) at debug.cpp:547:9 [opt]
    frame #6: 0x000000010010795c pyrogenesis`CCmpUnitMotionManager::Move(this=<unavailable>, ents=<no summary available>, dt=(value = <no summary available>)) at CCmpUnitMotion_System.cpp:434:3 [opt]
    frame #7: 0x000000010010513b pyrogenesis`CCmpUnitMotionManager::HandleMessage(this=<unavailable>, msg=<unavailable>, (null)=<no summary available>) at CCmpUnitMotion_System.cpp:0 [opt]
    frame #8: 0x0000000100192757 pyrogenesis`CComponentManager::BroadcastMessage(this=<no summary available>, msg=<unavailable>) at ComponentManager.cpp:1015:18 [opt]
    frame #9: 0x00000001000983b6 pyrogenesis`CSimulation2Impl::UpdateComponents(simContext=<no summary available>, turnLengthFixed=(value = <no summary available>), commands=<unavailable>) at Simulation2.cpp:566:20 [opt]
    frame #10: 0x0000000100096c76 pyrogenesis`CSimulation2Impl::Update(this=<no summary available>, turnLength=<no summary available>, commands=<unavailable>) at Simulation2.cpp:401:2 [opt]
    frame #11: 0x00000001001abeeb pyrogenesis`CTurnManager::Update(this=<unavailable>, simFrameLength=<no summary available>, maxTurns=<no summary available>) at TurnManager.cpp:146:17 [opt]
    frame #12: 0x0000000100207d95 pyrogenesis`CGame::Update(this=<no summary available>, deltaRealTime=<no summary available>, doInterpolate=<no summary available>) at Game.cpp:400:22 [opt]
    frame #13: 0x0000000100006cb4 pyrogenesis`RunGameOrAtlas(int, char const**) [inlined] Frame() at main.cpp:435:12 [opt]
    frame #14: 0x00000001000064c0 pyrogenesis`RunGameOrAtlas(argc=<no summary available>, argv=<no summary available>) at main.cpp:691 [opt]
    frame #15: 0x0000000100004ab6 pyrogenesis`main(argc=<no summary available>, argv=<no summary available>) at main.cpp:743:2 [opt]
    frame #16: 0x00007fff6ccebcc9 libdyld.dylib`start + 1

bisect

[26680]

issue

there is something wrong with the viking_longship.xml, it does not like the changes being made to depth if you simply revert back to the old number or use a number below 44 it works.

referenced line

The line under Assertion failed was added with [25182].

possible solutions

(1)

• change the depth of the viking_longship.xml or

(2)

• Double the PUSHING_GRID_SIZE ​CCmpUnitMotion_System.cpp or

Line
42	/**
43	* Units push within their square and neighboring squares (except diagonals). This is the size of each square (in meters).
44	* I have tested grid sizes from 10 up to 80 and overall it made little difference to the performance,
45	* mostly, I suspect, because pushing is generally dwarfed by regular motion costs.
46	* However, the algorithm remains n^2 in comparisons so it's probably best to err on the side of smaller grids, which will have lower spikes.
47	* The balancing act is between comparisons, unordered_set insertions and unordered_set iterations.
48	* For these reasons, a value of 20 which is rather small but not overly so was chosen.
49	*/
50	constexpr int PUSHING_GRID_SIZE = 40;

(3) Changing the properties of the units_demo map

[Langbart]

referenced line

[Langbart]

will upload a patch later, the fix is only a change in the units_demo map

[Silier]

In 26775:

Fix unit demo map

Split from D4589

Differential revision: D4600
Original patch by: @Langbart
Fixes: #6486
Refs: #6116

[Langbart]

Replying to Silier:

In 26775:

Fix unit demo map

Split from D4589

Differential revision: D4600
Original patch by: @Langbart
Fixes: #6486
Refs: #6116

Some comments from elexis via IRC 12/Apr/22.

[13:45:55] elexis fixing a c++ crash by changing the map to not trigger the crash, ok
[14:09:05] elexis that crash is not fixed, its in c++
[14:09:16] elexis how fragile is it?
[14:09:31] elexis whats the least a mapper would have to do to create it, could players trigger it?
[14:09:41] elexis placing some entity too close to the map border?
[14:42:20] elexis one only needs to place an entity so close to the map border that its half width exceeds the mapsize?
[14:47:40] Langbart i will test it and then reopen the ticket
[14:49:00] elexis I mean someone could say it doesnt matter if the engine crashes if one provides wrong user input, but its a less solid alternative for sure
[14:49:16] elexis depending on how likely it is to trigger the crash
[14:49:32] elexis but it really looks more like someone didnt add a safeguard in c++
[14:50:24] elexis with bad luck someone could trigger the bug in a running match by placing a CC too close to the map border or something
[14:50:40] elexis viking ship doesnt seem particularly big IIRC
[14:51:05] elexis there are 3 tiles impassable terrain at the map border, so I would have expected that the entity must be larger

Testing

• A crash could not be recreated by building large objects (civic center or wonders ) near the border and playing with ships on naval maps.
• If one would pick an object, that sits close on the border of the units_demo map e.g. templates/units/iber/ship_fishing.xml file and play around with the depth (e.g. depth="1000.0"), errors could be produced and lots of units not showing up on the map, but no crash. See image below for more clarity.

The recent fix no longer makes this ticket a release blocker, but the underlying issue is not fixed. For that reason the ticket will be reopened.

[14:09:05] elexis that crash is not fixed, its in c++

[Langbart]

see comment above

[Silier]

If one would pick an object, that sits close on the border of the units_demo map e.g. templates/units/iber/ship_fishing.xml file and play around with the depth (e.g. depth="1000.0"), errors could be produced and lots of units not showing up on the map, but no crash. See image below for more clarity.

Sure. Any next row is determined by the max depth of all objects in current row. If you take rest of the map by this object, next row will be out of map.

[Silier]

therefore entities placed out of map borders would trigger c++ crash because they are out of grid

[Silier]

elexis fixing a c++ crash by changing the map to not trigger the crash, ok

then fix it

[Langbart]

Noted added under "to reproduce"

[Freagarach]

Pushing back.

[Vladislav Belov]

Another case where we can get the assertion: ​https://wildfiregames.com/forum/topic/109099-map-editor-crash-testing-map-errno-11/