Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

More DebrisTypes than MaximumDebris causes Desync #1165

Open
6 tasks done
Danielovich7 opened this issue Sep 23, 2023 · 5 comments
Open
6 tasks done

More DebrisTypes than MaximumDebris causes Desync #1165

Danielovich7 opened this issue Sep 23, 2023 · 5 comments
Labels
Vanilla bug Vanilla game bugs that are requested to be fixed

Comments

@Danielovich7
Copy link

Danielovich7 commented Sep 23, 2023

Description

If there is more debris listed in DebrisTypes= than there are number of debris listed in MaximumDebirs=, it causes desync. That was the desync issue in Red Alert 20XX pre-1.0.6b. Removing the too many debris in DebrisTypes= fixed the issue.

Conditions to reproduce

On a TechnoType, add more debris in DebrisTypes= than available MaximumDebris=. Make sure they are Voxel debris.

INI code

...
DebrisTypes=DBRIS1, DBRIS2
DebrisMaximums=1
...

Steps to reproduce

  1. See above

...

Expected behaviour

A warning in debug.log, maybe with somehow sanitized value which would not cause a desync.

Actual behaviour

It causes desync.

Additional context

No response

Checklist

  • The issue is not introduced by Phobos or any other engine extension, such as Ares, Kratos etc.
  • The issue wasn't fixed in the most recent version of Ares/Phobos yet.
  • I agree to elaborate the details if requested and provide thorough testing if the bugfix is implemented.
  • I added a very descriptive title to this issue.
  • I used the GitHub search and read the issue list to find a similar issue and didn't find it.
  • I have attached as much information as possible (screenshots, gifs, videos, debug and exception logs, etc).
@Danielovich7 Danielovich7 added the Vanilla bug Vanilla game bugs that are requested to be fixed label Sep 23, 2023
@Otamaa
Copy link
Contributor

Otamaa commented Sep 23, 2023

Game using index based on DebrisTypes items count , ofcourse it will desync because it will do out of bound array access for the DebrisMaximums especially when DebrisMaximums items count is less than DebrisTypes items count , so this is not an bug but user error , as you can see on this address 0x0007023C8 for the details.

@Metadorius
Copy link
Member

Metadorius commented Sep 23, 2023

Game using index based on DebrisTypes items count , ofcourse it will desync because it will do out of bound array access for the DebrisMaximums especially when DebrisMaximums items count is less than DebrisTypes items count , so this is not an bug but user error , as you can see on this address 0x0007023C8 for the details.

Well probably the report could've been a bit clearer, but regardless the point is that it should not just silently fail somewhere. The point is that there should be either a clear warning that this causes a desync or maybe even a sanitized value which would not. Or the game should outright refuse to work until that is fixed.

@Otamaa
Copy link
Contributor

Otamaa commented Sep 23, 2023

Game using index based on DebrisTypes items count , ofcourse it will desync because it will do out of bound array access for the DebrisMaximums especially when DebrisMaximums items count is less than DebrisTypes items count , so this is not an bug but user error , as you can see on this address 0x0007023C8 for the details.

Well probably the report could've been a bit clearer, but regardless the point is that it should not just silently fail somewhere. The point is that there should be either a clear warning that this causes a desync or maybe even a sanitized value which would not. Or the game should outright refuse to work until that is fixed.

well , i agree that it problably just fail or maybe should put warning somewhere .
should we do TechnoType evaluation like ares does ? so we can put more warnings in case user doing something that not suppose to be ,..

@Metadorius
Copy link
Member

Metadorius commented Sep 23, 2023

Since Ares is stale - it will probably be a good idea to do so. The behavior I see best is emitting a developer warning and automatically fixing the value to closest valid one.

@Otamaa
Copy link
Contributor

Otamaa commented Sep 23, 2023

Since Ares is stale - it will probably be a good idea to do so. The behavior I see best is emitting a developer warning and automatically fixing the value to closest valid one.

https://github.com/Ares-Developers/Ares/blob/4f1d929920aca31924c6cd4d3dfa849daa65252a/src/Misc/Invalidators.cpp#L65C35-L65C35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Vanilla bug Vanilla game bugs that are requested to be fixed
Projects
None yet
Development

No branches or pull requests

3 participants