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

DIRECTOR: Implement working of titleVisible property of window #4114

Closed
wants to merge 2 commits into from

Conversation

r41k0u
Copy link
Contributor

@r41k0u r41k0u commented Jul 16, 2022

This set of changes resolves the missing window properties mentioned in https://trello.com/c/g1BaUzXz/292-missing-window-properties

@djsrv
Copy link
Member

djsrv commented Jul 16, 2022

It doesn't look like this will work properly if you do this:

set the title of window "test" to "foo"
set the titleVisible of window "test" to false
set the title of window "test" to "bar"
set the titleVisible of window "test" to true

after which the title "bar" should be visible.

Either Window should override setTitle to update _titleTemp, or setTitleVisible should instead be implemented on the superclass MacWindow and something similar can be done there (which I think is the cleaner option).

@@ -194,6 +194,7 @@ class Window : public Graphics::MacWindow, public Object<Window> {
Common::String _currentPath;
Common::StringArray _movieQueue;
int16 _startFrame;
Common::String _titleTemp;
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed since it's no longer used.

}
}

void MacWindow::setTitleVisibility(bool visible) {
Copy link
Member

@djsrv djsrv Jul 19, 2022

Choose a reason for hiding this comment

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

I'd rename this method to MacWindow::setTitleVisible and make Director's Window::setTitleVisible override it.

Also move the definitions for _titleVisible and isTitleVisible to MacWindow instead of Window to eliminate the need for the redundant _isTitleVisible.

@@ -189,6 +189,12 @@ void Window::setStageColor(uint32 stageColor, bool forceReset) {
}
}

void Window::setTitleVisible(bool titleVisible) {
_titleVisible = titleVisible;
setTitleVisibility(titleVisible);
Copy link
Member

Choose a reason for hiding this comment

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

Once you make this method override MacWindow::setTitleVisible you can replace this line with MacWindow::setTitleVisible(titleVisible);

case kTheModal:
return getModal();
case kTheFileName:
break;
Copy link
Contributor

Choose a reason for hiding this comment

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

This introduces a case where the function can return without returning a value, which clang flags as a warning:

    C++      engines/director/lingo/lingo-object.o
engines/director/lingo/lingo-object.cpp:534:1: warning: non-void function does not return a value in all control paths [-Wreturn-type]
}
^

@sev-
Copy link
Member

sev- commented Sep 18, 2022

Any updates? @r41k0u

@sev-
Copy link
Member

sev- commented Jun 1, 2023

Closing for the sake of #5067

@sev- sev- closed this Jun 1, 2023
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.

4 participants