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

Remove useless warnings #2916

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

Conversation

MatusGuy
Copy link
Member

No description provided.

src/supertux/sector.cpp Outdated Show resolved Hide resolved
@tobbi
Copy link
Member

tobbi commented Apr 20, 2024

Well, IDK, I guess it makes sense to show this warning for people who don't know what they're doing.

But the detection code needs to get fixed as there are a lot of false positives.

@tylerandari13
Copy link
Contributor

Well, IDK, I guess it makes sense to show this warning for people who don't know what they're doing.

People who don't know what they're doing will probably see these warnings as a sign that they did something wrong, despite the fact that nothings actually going wrong, hence why these are "useless warnings".

@bruhmoent
Copy link
Member

bruhmoent commented Apr 20, 2024

"Changing a Sector's gravitational constant might have unforeseen side-effects: "
"Tried spawning Tux in solid matter. Compensating."

These warnings are literally worthless.

@tylerandari13
Copy link
Contributor

tylerandari13 commented Apr 20, 2024

"Changing a Sector's gravitational constant might have unforeseen side-effects: " "Tried spawning Tux in solid matter. Compensating."

These warnings are literally worthless.

The solid matter warning is nothing more than a nuisance, and the gravity warning has probably discouraged at least one person (me) from using the gravity script.

@pazkero
Copy link
Contributor

pazkero commented Jun 5, 2024

My opinion has no weight but I'll comment anyway just so this PR seems more lively. I sent this on IRC earlier anyway and it's not like this PR affected me.

Surprisingly, I agree with Tobbi on that one...
Also because design perspective ─ if you try to spawn Tux in solid matter, that's illegal. The game tries to compensate instead of outright crashing on your face or doing it anyway and rendering you stuck, but this also means actual spawn point won't be the specified one. (The other "beware side-effects" are the kind of notes which should be in some sort of documentation for level makers, but I'm pretty sure there wasn't anything like that)
(so personally I think these could be moved to outside the source code, but not entirely removed unless their reason no longer exists e.g. gravity constant changes are always predictable and do not affect the behavior besides what's obvious)
In other hand, if you had some sort of level validator, the validator could issue these warnings instead
I think that's how FreedroidRPG does it, actually. You run the validator whenever you want, and it informs of any potential problems with the maps which you should be aware of before publishing them.

Well, here, this PR is lively again, review/merge when 🥺🤣

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants