Skip to content

Add gui combobox missing functions #280

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

Merged
merged 26 commits into from
Sep 22, 2018
Merged

Add gui combobox missing functions #280

merged 26 commits into from
Sep 22, 2018

Conversation

FileEX
Copy link
Contributor

@FileEX FileEX commented Jul 29, 2018

Syntax

int guiComboBoxGetItemCount(element theElement)
bool guiComboBoxSetOpen(element theElement, bool state)
bool guiComboBoxIsOpen(element theElement)

guiComboBoxIsOpen return boolean, nil if bad argument
guiComboBoxSetOpen return boolean

@qaisjp qaisjp added the enhancement New feature or request label Jul 29, 2018
RUN_CHILDREN(GUIComboBoxGetItemCount(**iter))

// Are we a CGUI Element?
if (IS_GUI(&Entity))
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix the indentation here and below.

int state;
CScriptArgReader argStream(luaVM);
argStream.ReadUserData<CGUIComboBox>(comboBox);
argStream.ReadNumber(state);
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd personally prefer an enum string here (and ofc return a string in guiComboxGetState). I'd like to hear other opinions though.

Or, an even better solution - as you only distinguish two states - is to rename the function to guiComboxIsOpen and make it return a boolean.

if (IS_GUI(&Entity))
{
CClientGUIElement& GUIElement = static_cast<CClientGUIElement&>(Entity);
int comboState;
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip: Try reducing the scope of variables as much as possible, i.e. move comboState into the if block or avoid creating it entirely by directly returning from the vs / !vs - if blocks.

@FileEX
Copy link
Contributor Author

FileEX commented Jul 31, 2018

Everything fixed

@qaisjp qaisjp added this to the 1.5.6 milestone Aug 10, 2018
@@ -5189,6 +5189,72 @@ bool CStaticFunctionDefinitions::GUIComboBoxSetItemText(CClientEntity& Entity, i
return false;
}

int CStaticFunctionDefinitions::GUIComboBoxGetItemCount(CClientEntity& Entity)
{
RUN_CHILDREN(GUIComboBoxGetItemCount(**iter))
Copy link
Contributor

Choose a reason for hiding this comment

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

RUN_CHILDREN is unnecessary in Get methods


bool CStaticFunctionDefinitions::GUIComboBoxIsOpen(CClientEntity& Entity)
{
RUN_CHILDREN(GUIComboBoxIsOpen(**iter))
Copy link
Contributor

Choose a reason for hiding this comment

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

As above, RUN_CHILDREN is unnecessary in Get methods

@@ -241,6 +241,11 @@ void CGUIComboBox_Impl::Clear(void)
m_pWindow->setText(storedCaption);
}

bool CGUIComboBox_Impl::GetState(void)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be renamed to IsOpen

@qaisjp qaisjp removed this from the 1.5.6 milestone Sep 2, 2018
@patrikjuvonen
Copy link
Contributor

@qaisjp @Jusonex What's the status on these reviews?

@patrikjuvonen patrikjuvonen added this to the 1.5.7 milestone Sep 5, 2018
@qaisjp qaisjp removed this from the 1.5.7 milestone Sep 7, 2018
@qaisjp qaisjp merged commit 5d682ba into multitheftauto:master Sep 22, 2018
@qaisjp qaisjp added this to the 1.5.7 milestone Sep 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants