Introducing the Official community mod for 0 A.D. Empires Ascendant

[Philip the Swaggerless]

1 hour ago, Norse_Harold said:

No, but send replays, please. The simpler and shorter, the better.

Here is the reply commands portion.  no metadata was saved though...

By the way, I just played some single player games and had no issue.

commands.txt

[real_tabasco_sauce]

possible lead:

https://gitea.wildfiregames.com/0ad/0ad-community-mod-a26/pulls/4/files#diff-f8ba294e053371dce7a50759dae18522c3e0c09a:~:text=out the way-,// out the way,},-// The obstruction always

 

[Norse_Harold]

If IsFinished() is called more than once on a foundation that is at buildprogress of 1.0 then obstructing entities can be destroyed more than once. Unexpected results may occur.

I added some logging statements and confirmed that entities are getting destroyed more than once.

I think that the solution is to clear entsToDestroy after the entities in it are destroyed.

I've attached a patch that demonstrates this.

0ad-community-mod-02611-maybefixed.patch

[Norse_Harold]

Another possibly unwanted behavior that I notice is that obstructing entities that would normally be destroyed at the moment that the foundation is created will instead be delayed and only destroyed after the building is finished. I wonder if this could disrupt pathfinding when units are standing on the foundation at the time that construction is intended to begin.

A solution to this problem would be to only delay destruction of obstructing entities if the structure to be built is a wall and the obstructing entities are trees.

[real_tabasco_sauce]

18 minutes ago, Norse_Harold said:

If IsFinished() is called more than once on a foundation that is at buildprogress of 1.0 then obstructing entities can be destroyed more than once. Unexpected results may occur.

I added some logging statements and confirmed that entities are getting destroyed more than once.

I think that the solution is to clear entsToDestroy after the entities in it are destroyed.

I've attached a patch that demonstrates this.

0ad-community-mod-02611-maybefixed.patch 698 B · 0 downloads

it would be interesting if this fixes the problem. Many of the problems have occurred when building over no entities, ie just building a farmstead when expected.

[Norse_Harold]

48 minutes ago, real_tabasco_sauce said:

it would be interesting if this fixes the problem. Many of the problems have occurred when building over no entities, ie just building a farmstead when expected.

In my testing, I was seemingly building over no entities, but I think that there are less visible or invisible entities that are being automatically deleted when buildings are constructed there.

Also, I see your proposed changes in the fix_crash branch to re-add `return false` in Commit in Foundation.js. This is good. I think that it will correctly prevent building placement if there is an obstructing entity that is not caught by BuildRestrictions.js:CheckPlacement, such as an entity that moves into the area between the time that CheckPlacement() is called and Commit() is called. An example would be another structure being placed there in that time. This is a guess, though. I don't know whether there is actually a race condition there. Edit: Actually, I think that the purpose of the call to GetEntitiesBlockingConstruction() is to check whether there are units blocking construction and order them to move out of the way.

I think that a correct solution would involve all three changes

• apply my patch to clear entsToDestroy after the entities in it are destroyed
• re-add return false on line 275 of Foundation.js, in Commit(), if there are obstructing entities. However, if the building to construct is a wall and the obstructing entities are only trees then do not return false in that case
• in Commit at line 259, immediately destroy all obstructing entities that would be destroyed on construction except trees. If the entity to be built is a wall then queue trees for later destruction

Some of the code would seem to be redundant. Is there a way to move the wall/tree code block into a separate function so that it can be used by CheckPlacement() and Commit()?

Edited September 10, 2024 by Norse_Harold

[real_tabasco_sauce]

2 minutes ago, Norse_Harold said:

In my testing, I was seemingly building over no entities, but I think that there are less visible or invisible entities that are being automatically deleted when buildings are constructed there.

Also, I see your proposed changes in the fix_crash branch to re-add `return false` in Commit in Foundation.js. This is good.

I think that a correct solution would involve all three changes

• apply my patch to clear entsToDestroy after the entities in it are destroyed
• re-add return false in Commit if there are obstructing entities
• immediately destroy all obstructing entities that would be destroyed on construction except trees. If the entity to be built is a wall then queue trees for later destruction

Well restoring return false was problematic as it would result in endless construction when building a wall segment on top of a tree.

That being said, I was unable to cause the crashing after this change in 2 multiplayer games.

[real_tabasco_sauce]

@Norse_Harold what were your steps to try and reproduce the crash? Did you get any crashes with the patch you shared? lmk if you want to test something.

Edited September 10, 2024 by real_tabasco_sauce

[real_tabasco_sauce]

Also, did anyone observe a crash in a game where nobody played as romans?

Edited September 10, 2024 by real_tabasco_sauce

[Norse_Harold]

1 minute ago, real_tabasco_sauce said:

@Norse_Harold what were your steps to try and reproduce the crash?

I mimiced the Romans player in PhiliptheSwaggerless's replay. I was not able to reproduce the crash. But I only intended to cause multiple destroy calls per entity, and I was successful in that.

To see that, use the attached Foundation.js file which enables logging. Then build lots of structures and watch the logs for multiple occurrences of "destroying entity #", where # is the same number for each occurrence. It may help to build structures with lots of units and on top of grass actors.

Foundation.js

[real_tabasco_sauce]

3 minutes ago, Norse_Harold said:

I mimiced the Romans player in PhiliptheSwaggerless's replay. I was not able to reproduce the crash. But I only intended to cause multiple destroy calls per entity, and I was successful in that.

To see that, use the attached Foundation.js file which enables logging. Then build lots of structures and watch the logs for multiple occurrences of "destroying entity #", where # is the same number for each occurrence. It may help to build structures with lots of units and on top of grass actors.

Foundation.js 14.55 kB · 0 downloads

Were you in multiplayer? The crashes only seem to happen in multiplayer with a player opponent or spectator.

[Norse_Harold]

Just now, real_tabasco_sauce said:

Were you in multiplayer? The crashes only seem to happen in multiplayer with a player opponent or spectator.

I was not in multiplayer, but if my theory is correct that excessive destroy calls on the same entity are the cause of the crash then it's a useful line of investigation.

[real_tabasco_sauce]

Well, we could just scrap the 'walls delete trees' for the time being.

[Norse_Harold]

3 minutes ago, real_tabasco_sauce said:

Well, we could just scrap the 'walls delete trees' for the time being.

Yes, we could, although I would like that it be made into a separate mod so that people can do testing and development of it.

[Norse_Harold]

This discussion about the bug in the 'walls delete trees' feature was probably better done in an issue on the Gitea repository 0ad-community-mod-a26. Next time...

Edited September 10, 2024 by Norse_Harold

[real_tabasco_sauce]

5 minutes ago, Norse_Harold said:

Yes, we could, although I would like that it be made into a separate mod so that people can do testing and development of it.

@Grapjas made the original:

https://wildfiregames.com/forum/topic/18003-suggestions-for-0-ad/?do=findComment&comment=579557

[Stan`]

I haven't looked at the code. But the crash is due to something not being serializable (the serialization error from above). Could be a NaN value for instance from the error it seems total builder is NaN

 

[real_tabasco_sauce]

Ok I can confirm it only seemed like a roman thing because lots of ppl were playing romans. I just reproduced it with other civs.

[Stan`]

18 minutes ago, real_tabasco_sauce said:

Ok I can confirm it only seemed like a roman thing because lots of ppl were playing romans. I just reproduced it with other civs.

Did you find a consistent way to reproduce it ?

[real_tabasco_sauce]

the best way to do it is just to get in a 1v1, don't use gift from the gods, and start building a lot of buildings.

I can't reproduce it by building any specific building in any specific place, I just build until it crashes.

[real_tabasco_sauce]

community-testing-5.zip

these changes add @Norse_Harold's above suggestion to prevent entities from being deleted more than once, and includes the changes to only return false; if the building is not a wall or palisade.

This prevented any crashes until we started building walls. We did a game where we only placed walls on completely bare ground, completely avoiding trees and decorative entities, and the crash still occurred.

Edited September 10, 2024 by real_tabasco_sauce

[Norse_Harold]

1 hour ago, real_tabasco_sauce said:

This prevented any crashes until we started building walls. We did a game where we only placed walls on completely bare ground, completely avoiding trees and decorative entities, and the crash still occurred.

Good idea to run that test. That's interesting that the crash still occurred. I advise separating the walls delete trees mod from the rest of community-mod for further testing, so that a binary search algorithm can be applied where the search space is cut in half with each iteration.

If the problem is then found to be correlated with the walls delete trees mod then here are some things to consider.

• Normally (invisible or less visible) obstructions would be destroyed before the foundation is committed instead of after it's finished.
• Maybe isFinished isn't called after every structure is completely constructed. In that case, obstructing entities aren't destroyed.
• Units blocking construction might conflict with or be locked into the structure if return false isn't present on line 275 of Foundation.js.

Edited September 10, 2024 by Norse_Harold

[real_tabasco_sauce]

recently, I tried another conditional return, only when totalBuildRate was NaN. This didn't help, because totalBuildRate presumably becomes NaN after commit().

			if (isNaN(this.totalBuilderRate)) {
				warn("Value of builder rate is NaN");
				return false;
			}

I wonder if there is something we can check instead of totalBuildRate that will return false when we want it to.

Maybe EntstoDestroy being empty or something else?

Edited September 10, 2024 by real_tabasco_sauce

[Stan`]

Somewhere somehow you're doing an operation where you do undefined + number or null + number or nan + number or sometimes string + number

[real_tabasco_sauce]

I'm inclined to think that this was a pre-existing error that was just uncovered by the building on trees mod. Maybe some amount of buildings had this issue, but the return false; was catching them.