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

Fix endless pressure bug. The changes regard the second amplifying ef… #622

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

EchoHowardLam
Copy link
Contributor

…fect. Original code directly sets velocity to 0, which creates velocity difference from nowhere.

…fect. Original code directly sets velocity to 0, which creates velocity difference from nowhere.
@ghost
Copy link

ghost commented Jan 7, 2019

Relevant issue: #580

@EchoHowardLam
Copy link
Contributor Author

Ok thanks for tagging, I forgot that.

@jacob1
Copy link
Member

jacob1 commented Jan 7, 2019

ok, cool. I will test this more later. I'm not sure why "bmap_blockair[y][x+1]" has to exist at all

@EchoHowardLam
Copy link
Contributor Author

Note that this is still kind of a hotfix instead of a design fix, so I limit the change to be minimal. It would be better to redesign the code to take care of the inconsistency appearing over the air code.

@EchoHowardLam
Copy link
Contributor Author

Hold on a bit, bmap_blockair[y][x+1] is responsible for avoiding pressure leaking thro walls. Now it seems that the convolution actually allows pressure to sneak into walls so this change will cause pressure leak.

@jacob1
Copy link
Member

jacob1 commented Jan 9, 2019

If it helps, you could try changing it so pressure doesn't go into walls. Right now it goes into the wall but doesn't go through to the other side.

@ghost
Copy link

ghost commented Jan 9, 2019

That would be preferable. It's weird how deleting walls can trigger a release of pressure. TTAN has the same problem.

@EchoHowardLam
Copy link
Contributor Author

EchoHowardLam commented Jan 10, 2019

I don't quite agree to completely remove pressure inside wall, otherwise TTAN container within a grid's size cannot store any pressure. Instead, the problem is on how pressure outside the wall can affect pressure inside. Anyway, I would attempt this pressure blockage when I have time, but that would be another issue.

@jacob1
Copy link
Member

jacob1 commented Jan 10, 2019

Yes, that would be a breaking change to TTAN, but I've seen complaints that TTAN doesn't properly block pressure because people are confused the pressure still goes into TTAN's cell anyway.

Anyway it was more of a suggestion in case that makes it easier. But changing that is probably not easier.

I might have some time to look into this bug myself next week. But my main focus is getting 94.0 released when I have time to do that.

@EchoHowardLam
Copy link
Contributor Author

EchoHowardLam commented Jan 10, 2019

After some more code digging, it is found that this second amplifying effect is partially affected by the asymmetry of how positive wave and negative wave performs. Negative wave have difficulty to do reflection on walls that the reflected wave is mostly positive rather than being negative wave. This effect could not be explained by the code above the convolution so I looked further downwards, which I found some missing checking that leads to bilinear sampling INTO walls. The early stage of endless pressure can still happen but it could not last long.

PS: For identifying strength of negative wave, you can press 'i' after pausing. It is quite obvious that positive wave reflects with more preserved energy than negative wave.

@EchoHowardLam EchoHowardLam reopened this Jan 10, 2019
@moonheart08
Copy link
Contributor

Looking through aircode, it might be a good safety measure to replace the various instances of

(int)(anumberbeingrounded + 0.5f)

with std::round if the number cannot be guaranteed positive. The issue being that the +0.5f method does not function properly on negative numbers.

I'm not sure, but this may be a potential hidden reason behind inconsistencies between positive and negative directions.

@LBPHacker LBPHacker added Enhancement implemented but could be better Work in progress ignore this for now labels Dec 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement implemented but could be better Work in progress ignore this for now
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants