Add interact state to fix invisible lasers
This is a ~~work in progress~~ refactor for projectile visibility and interact state to solve https://github.com/ddnet/ddnet/issues/9446 and similar issues. The core problem is that projectiles use the owner character to lookup state. But the owner might be dead or disconnected or switch ddrace teams and similar.
The proposed fix is to store all state needed to determine if a projectile can hit or be seen directly on the projectile. It still queries the full state every tick from the character to not change any old behavior even if it was weird. This can still be done in a feature refactor. When the owner disconnects it just keeps the old state instead of resetting to some unset state as it is doing right now.
~~The code is work in progress and just an idea for now. It still needs to be finished and polished. Because this is time consuming I would like to have some maintainer feedback before I put a lot of time into this refactor.~~
After 2 weeks of no feedback I fully finished and tested the pr. Works fine and is ready to be merged.
This pr does not fix grenades or id reuse: #10398, #9446 but lays some good foundation for it
This pr already fixes #10417, #9229
https://github.com/user-attachments/assets/067fb779-b8fc-494d-800a-b214e5a644d1
This can still be done in a feature refactor.
Do you intend to do that refactor or are you just remarking that anything can be done by changing code?
Do you intend to do that refactor or are you just remarking that anything can be done by changing code?
Yes I plan to fix more once this is merged
I would love to see some progress here! Anything specific blocking here? Or why has nobody reviewed it yet? Is the fix-changes-physics label too scary? I can also exclude the fix of #10417 and remove the physics change and keep it as just a visual fix for now if it helps the merge.
The proposed fix is to store all state needed to determine if a projectile can hit or be seen directly on the projectile. It still queries the full state every tick from the character to not change any old behavior even if it was weird. […] When the owner disconnects it just keeps the old state instead of resetting to some unset state as it is doing right now.
This sounds like a really good idea, I like it. :) It makes the code more explicit about its intention. :)
I'm unsure about the name InteractState, but if we find a better name for it, we can just search-replace it. :)
It looks sane, it feels sane, hard to make sure it didn't change anything. At least a cursory glance suggests it didn't. @heinrich5991 does this need a teehistorian bump, just in case if it changed something accidentally?
That would be a question for @Zwelf or @Patiga.
It changed something intentionally and does require a teehistorian bump! See https://github.com/ddnet/ddnet/pull/10399#issuecomment-3101236726
What is the plan here? How do I get this merged?
Any updates?
Any update here? Can I do something to progress the merge?
@Robyt3 can you take a look please? This is open for half a year now and I don't see why this is not merged yet
github is hiding the reply box gg
https://github.com/ddnet/ddnet/pull/10399#discussion_r2514438269
The plan is to not maintain duplicated code. But have the new class as source of truth it should be called for everything. Its not cut out yet to keep the pr thinner while its stuck in review. This pr is only the start of the cleanup.
Amazing, I have the same github bug.
Ah, nevermind, it's probably because it's an earlier comment thread.