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

Make Show fallback to global variables when sensible (bug #5278) #2694

Merged
merged 2 commits into from
Feb 9, 2020

Conversation

Capostrophic
Copy link
Collaborator

Bug report.

In case there isn't a local variable for the given object, Show will try to find a global namesake.

The way stringstream emptiness is detected is somewhat clunky, but I don't want to actually recover the string that is put into it before it's actually necessary.

@@ -923,11 +923,7 @@ namespace MWScript
if (!ptr.isEmpty())
{
const std::string& script = ptr.getClass().getScript(ptr);
if (script.empty())
{
output << ptr.getCellRef().getRefId() << " has no script " << std::endl;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do a user see a difference is it a global variable or a script variable?

Copy link
Collaborator Author

@Capostrophic Capostrophic Feb 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have now changed the format to the way vanilla does it with slight deviations (a member separator, dot instead of ->, the case of the original variable name isn't preserved). Hopefully it's clear enough.

default:
output << "unknown local '" << var << "' for '" << ptr.getCellRef().getRefId() << "'";
break;
// Do nothing otherwise
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Useless comment. It's obvious that there is nothing else in this switch statement.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arguably it demonstrates to the reader that all possible switch cases were considered. Ok, I'll remove it.

@elsid elsid merged commit 158f610 into OpenMW:master Feb 9, 2020
@Capostrophic Capostrophic deleted the show branch February 9, 2020 19:56
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.

2 participants