Skip to content

Fix for issue #377 (many collisionless objects) #378

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

Merged
2 commits merged into from
Sep 3, 2018

Conversation

samr46
Copy link
Contributor

@samr46 samr46 commented Sep 2, 2018

I also suffering from this issue #377 for a few years now (dozens collisionless objects due replacement of some models in game).

It looks like CFileLoader_LoadCollisionFile_Mid hook causing this problem. We don't need to skip loading GTA collision model if we have replaced it. It's unnecessary because SetColModel is called.

I didn't remove this hook for now (just commented installation). It needs some testing. It would be really good if somebody else with these problems could confirm the fix and that collision replacement works properly.

Also TestResourceBug2.zip (object: 5418) from Issue description always causes this problem (at least for me).

This PR closes #377

CFileLoader_LoadCollisionFile_Mid is unnecessary and causing problems (not removed; just commented for now)
@patrikjuvonen patrikjuvonen added the bug Something isn't working label Sep 2, 2018
@ccw808
Copy link
Member

ccw808 commented Sep 2, 2018

CFileLoader_LoadCollisionFile_Mid was added here to 'Fix custom object collisions getting randomly reset'
Will that problem not happen now?

@samr46
Copy link
Contributor Author

samr46 commented Sep 2, 2018

I tested it with custom objects too and never noticed any problem. But if it's random issue and it was a fix then technically problem should return. This is why it needs more testing.

CFileLoader_LoadCollisionFile_Mid calls on object streamIn. First i thought that it would reset model to original so i tried to stream out / in and it worked fine in all cases (different custom and world objects).
Maybe there is some specific way to reproduce this old bug?

@Einheit-101
Copy link

I suggest implementing it in the latest nightly and i will test if any collision issue appears until i die. Because i suffer a lot.

@ArranTuna
Copy link
Collaborator

The bug that has caused us such grief, can be fixed by "//"? :/

Same as Einheit, put in latest nightly and it will be tested thoroughly.

This bug has prevented us from adding many new objects that we have wanted to add, so as well as fixing a bug that causes problems for players every day this will open up whole new possibilites.

@samr46
Copy link
Contributor Author

samr46 commented Sep 3, 2018

Well i don't want to remove this hook and dependencies completely for now. "//" should be enough for testing.
On my servers i added fake collisions to cover these holes. But this is really annoying and doesn't solve all problems.

Like @ccw808 mentioned this hook was added for a reason. I can't reproduce this old bug to test everything properly so there is a chance that the old problem can come up due this solution.
Anyway i think it's worth a try. I already tired of reports about collisionless objects.

@ccw808
Copy link
Member

ccw808 commented Sep 3, 2018

How to check old bug fix:
Code:

  • Change OnMY_CFileLoader_LoadCollisionFile_Mid to return TRUE and also print a message showing which collision model is being loaded

In game:

  • Replace a collision model with something vastly different (to make it easier to tell if the collision has been reset back to the original)
  • Move about until the message saying GTA has reloaded the collision model
  • Check collision has not been reset

@ccw808
Copy link
Member

ccw808 commented Sep 3, 2018

If the old bug can't be reproduced, then we should merge this for general testing

@samr46
Copy link
Contributor Author

samr46 commented Sep 3, 2018

I re-checked it several times. Everything looks good.
Hook and all relevant code is removed. We can merge this for general testing.

@ccw808 ccw808 self-requested a review September 3, 2018 21:22
Copy link
Member

@ccw808 ccw808 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge

@ghost ghost changed the base branch from master to next September 3, 2018 22:54
@ghost ghost merged commit 36b593d into multitheftauto:next Sep 3, 2018
@patrikjuvonen patrikjuvonen added this to the 1.5.7 milestone Sep 4, 2018
@patrikjuvonen patrikjuvonen assigned ccw808 and ghost Sep 4, 2018
@Brian9221
Copy link

I've tested this bug fix this morning, seems that everything is fine and without bugs, finally! :)

Stream in/out: Works extremely well, no collisions bugs on replaced models
Restarting resource with obj streamed in: It is replaced without bugs

I hope this important fix is implemented as soon as possible.

@Einheit-101
Copy link

I am very sorry to say this, but i have discovered random collisionless objects after testing this for a while now, the difference is that we cannot reproduce it anymore with the test resource. Can someone else confirm?

https://i.imgur.com/SLpeuJW.png

@Brian9221
Copy link

I have tested that object on an heavily modded server and it doesn't appear as bugged. Maybe it's your script interfering?

@Einheit-101
Copy link

Einheit-101 commented Oct 21, 2018

No, it appears random, you cannot reproduce it. On another day some other objects will be bugged. I replaced many collisions on my server and the more you replace, the higher the chance for a bug.
BUT it is not as bad as before, it seems a lot better.

@samr46
Copy link
Contributor Author

samr46 commented Oct 21, 2018

I can't confirm this. I use this object for different elevators for a long time. Players use it every day.

Some statistics from my servers:
~400 replaced collisions in the gamemode.
Before the fix: 2-7 reports about collisionless objects on the map per day.
After the fix: 0 reports.

@Wolfee-J
Copy link

Does this fix the bug with SA objects loosing collisions?

@ccw808
Copy link
Member

ccw808 commented Oct 21, 2018

@Einheit-101 Set Advanced->Streaming memory to Min and see how that affects this issue

1 similar comment
@ccw808
Copy link
Member

ccw808 commented Oct 21, 2018

@Einheit-101 Set Advanced->Streaming memory to Min and see how that affects this issue

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants