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

ULTIMA8: Custom type checking cleanup #2244

Merged
merged 3 commits into from May 13, 2020

Conversation

@OMGPizzaGuy
Copy link
Contributor

OMGPizzaGuy commented May 12, 2020

ULTIMA8: Custom type checking cleanup

Use template methods for type checking instead of referencing static ::ClassType variable directly.
Alter Gump::FindGump to check matches against a function instead of referencing the RunTimeClassType and remove no_inheritence option as it is never used. These changes should help migration from p_dynamic_cast if desired.

OMGPizzaGuy added 2 commits May 12, 2020
…tead of referencing the RunTimeClassType

Removed no_inheritence option as it is never used. These changes should help migration from p_dynamic_cast if desired
…g static ::ClassType variable directly
@@ -108,23 +108,20 @@ class Gump : public Object {
//! \param takefocus If true, set parent's _focusChild to this
virtual void InitGump(Gump *newparent, bool take_focus = true);

//! Find a gump of the specified type (this or child)
//! \param t Type of gump to look for
typedef bool (Gump::*FindPredicate)() const;

This comment has been minimized.

Copy link
@OMGPizzaGuy

OMGPizzaGuy May 12, 2020

Author Contributor

Should I consider a Common::UnaryFunction for this instead of member function pointer?

This comment has been minimized.

Copy link
@mduggan

mduggan May 13, 2020

Contributor

I don't have a strong feeling on this either way. Since the usage is fairly limited it may be easier to just do it like this?

@OMGPizzaGuy
Copy link
Contributor Author

OMGPizzaGuy commented May 12, 2020

Another use of RunTimeClassType is for debugging information - specifically for the class name - in a number of console commands through the code. I'm curious to know what thoughts are on these commands.

It also appears to be used on save objects and processes, but load class names appears to be hard coded for object manager and the kernel's process loaders.

@OMGPizzaGuy OMGPizzaGuy changed the title U8 type check cleanup ULTIMA8: Custom type checking cleanup May 12, 2020
@OMGPizzaGuy
Copy link
Contributor Author

OMGPizzaGuy commented May 13, 2020

The "bool IsOfType(const char * type)" functions are currently unused and I'm considering removing them as well.

engines/ultima/ultima8/gumps/gump.cpp Outdated Show resolved Hide resolved
@mduggan
Copy link
Contributor

mduggan commented May 13, 2020

Another use of RunTimeClassType is for debugging information - specifically for the class name - in a number of console commands through the code. I'm curious to know what thoughts are on these commands.

I think now that most of the stuff is explicitly defined in the debugger class, the usefulness of this is lower than it was in Pentagram?

It also appears to be used on save objects and processes, but load class names appears to be hard coded for object manager and the kernel's process loaders.

Yeah, the save/load stuff uses the names hard coded there. I know because I broke my savegames by renaming the music class and had to fix it :)

@mduggan
Copy link
Contributor

mduggan commented May 13, 2020

Thanks for doing this cleanup! I'm also in favor of removing the unused code. Since it's just on the U8 engine we probably don't need to leave open for comments so I'll merge now.

@mduggan mduggan merged commit d56b63f into scummvm:master May 13, 2020
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.