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

Bye Warning C4266 #4453

Merged
merged 2 commits into from
Sep 6, 2023
Merged

Bye Warning C4266 #4453

merged 2 commits into from
Sep 6, 2023

Conversation

MillhioreBT
Copy link
Contributor

Pull Request Prelude

  • I have followed [proper The Forgotten Server code styling][code].
  • I have read and understood the [contribution guidelines][cont] before making this PR.
  • I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

I'm not sure, but some expert can say if it's correct.
I'm using 'using' to specify the use of base methods, so the compiler doesn't issue warnings anymore.

Notes

We just need to get rid of the C4244 warning.
It's quite bothersome because not all of them show up, so we have to compile multiple times and discover them one by one.

Issues addressed: Nothing!

EPuncker
EPuncker previously approved these changes May 14, 2023
@ranisalt
Copy link
Member

Not sure this is the correct solution, but at least these methods should be moved to the top (right after constructors)

@MillhioreBT
Copy link
Contributor Author

Not sure this is the correct solution, but at least these methods should be moved to the top (right after constructors)

The other option is to create the method and call the parent method explicitly.

void onWalk(...) override { Creature::onWalk(...); }

I don't know which is better, but using seems simple to me.
honestly didn't know that using could be used for this.

@MillhioreBT MillhioreBT added the priority: low Issues with this label won’t have the immediate focus label May 28, 2023
@Erza
Copy link
Contributor

Erza commented Sep 5, 2023

Not sure this is the correct solution, but at least these methods should be moved to the top (right after constructors)

The other option is to create the method and call the parent method explicitly.

void onWalk(...) override { Creature::onWalk(...); }

I don't know which is better, but using seems simple to me. honestly didn't know that using could be used for this.

I ran into this too, that's the solution I went with, but it's super stupid that we even have to do this.
There is also an insane amount of Warning C4242 in Visual Studio, e.g. warning C4242: 'argument': conversion from 'uint32_t' to 'uint16_t', possible loss of data

I've spent hours addressing these, but I feel like whenever I fix like 20 of them, 30 new instances show up in the next build. What happened that these suddenly started showing up? The last time the warning level for Visual Studio was adjusted was by Mark in 2013. https://github.com/otland/forgottenserver/blame/master/vc17/theforgottenserver.vcxproj#L145

@MillhioreBT MillhioreBT merged commit daef35e into otland:master Sep 6, 2023
@MillhioreBT MillhioreBT deleted the bye_warnings branch September 6, 2023 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low Issues with this label won’t have the immediate focus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants