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.

Sign up for GitHub

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

Jump to bottom

Command list improvs #357

Merged
merged 1 commit into from May 6, 2021
Merged

Command list improvs #357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
267 changes: 146 additions & 121 deletions DROD/CharacterDialogWidget.cpp
View file
Open in desktop
Expand Up @@ -26,6 +26,7 @@
* ***** END LICENSE BLOCK ***** */

#include "CharacterDialogWidget.h"
#include "CommandListBoxWidget.h"
#include "DrodBitmapManager.h"
#include "DrodFontManager.h"
#include "EditRoomScreen.h"
Expand Down Expand Up @@ -189,6 +190,9 @@ const UINT LIST_LINE_HEIGHT = 22;
const SURFACECOLOR PaleRed = {255, 192, 192};

std::map<UINT, UINT> CCharacterDialogWidget::speechLengthCache;
const UINT CCharacterDialogWidget::INDENT_PREFIX_SIZE = 8;
Copy link
Member

Choose a reason for hiding this comment

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

These constants should be consolidated with the analogous magic numbers featured elsewhere in the code, e.g., CCommandListBoxWidget.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mrimer I'm not quite sure what needs to be consolidated where. These constants appear to be grouped with the other ones in this file, and they don't appear to share a purpose with the ones in CCommandListBoxWidget. That file does have some constants littered about, but they are in the place where they're being used, so it doesn't seem like much of an issue.

Copy link
Member

Choose a reason for hiding this comment

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

I appreciate you looking into this. I possibly misinterpreted the values. IIRC, to me, it looked like there are identical dimension values in both places, e.g., 8 pixels for an indent size, that possibly need to remain in lockstep in both places for things to work correctly. If this isn't the case, I'm fine with leaving things as-is for now.

const UINT CCharacterDialogWidget::INDENT_TAB_SIZE = 3;
const UINT CCharacterDialogWidget::INDENT_IF_CONDITION_SIZE = 5;

#define NOT_FOUND (UINT(-1))

Expand Down Expand Up @@ -493,8 +497,8 @@ CCharacterDialogWidget::CCharacterDialogWidget(

AddWidget(new CLabelWidget(0L, X_COMMANDSLABEL, Y_COMMANDSLABEL,
CX_COMMANDSLABEL, CY_COMMANDSLABEL, F_Small, g_pTheDB->GetMessageText(MID_Commands)));
this->pCommandsListBox = new CListBoxWidget(TAG_COMMANDSLISTBOX, X_COMMANDS, Y_COMMANDS,
CX_COMMANDS, CY_COMMANDS, false, true, true);
this->pCommandsListBox = new CCommandListBoxWidget(TAG_COMMANDSLISTBOX, X_COMMANDS, Y_COMMANDS,
CX_COMMANDS, CY_COMMANDS);
AddWidget(this->pCommandsListBox);

//Appearance (character/tile graphic).
Expand Down Expand Up @@ -2462,7 +2466,7 @@ void CCharacterDialogWidget::OnKeyDown(
line!=selectedLines.end(); ++line)
{
if (*line < pCommands->size())
wstrCommandsText += toText(*pCommands, (*pCommands)[*line]);
wstrCommandsText += toText(*pCommands, (*pCommands)[*line], *line);
}
if (!wstrCommandsText.empty())
g_pTheSound->PlaySoundEffect(SEID_POTION);
Expand Down Expand Up @@ -3228,8 +3232,6 @@ const
{
WSTRING wstr;

wstr += GetPrettyPrinting(commands, pCommand, 6, 3); //indent the If conditions considerably

//Call language-specific version of method.
switch (Language::GetLanguage())
{
Expand Down Expand Up @@ -4066,158 +4068,176 @@ WSTRING CCharacterDialogWidget::GetDataName(const UINT dwID) const
}

//*****************************************************************************
WSTRING CCharacterDialogWidget::GetPrettyPrinting(
//Add indenting to clarify the code flow.
//
//Params:
const COMMANDPTR_VECTOR& commands,
CCharacterCommand* pCommand,
const UINT ifIndent, const UINT tabSize)
const
UINT CCharacterDialogWidget::ExtractCommandIndent(const UINT wCommandIndex) const
//Extracts command's indent size from the command listbox text
{
ASSERT(pCommand);
WSTRING wstr = this->pCommandsListBox->GetTextAtLine(wCommandIndex);

WSTRING wstr;
UINT i = 0;
for (; i < wstr.size(); ++i)
{
if (!iswspace(wstr.at(i)))
break;
}

//Determine indentation for command.
if (pCommand->command == CCharacterCommand::CC_Label) //labels always have indent 0
return wstr;
return i;
}

//*****************************************************************************
void CCharacterDialogWidget::PrettyPrintCommands(const COMMANDPTR_VECTOR &commands)
{
WSTRING wstr;

UINT wNestDepth = 0, wIndent = 2; //insert past labels
UINT wNestDepth = 0, wIndent = INDENT_PREFIX_SIZE; //insert past labels

bool bIfCondition = false;
UINT index = 0;
bool bLastWasIfCondition = false;
for (COMMANDPTR_VECTOR::const_iterator command = commands.begin();
(*command) != pCommand && command != commands.end(); ++command)
command != commands.end(); ++command, ++index)
{
bIfCondition = false;
switch ((*command)->command)
wstr.clear();
CCharacterCommand *pCommand = *command;
bool bIsIfCondition = false;
bool bUndoOneDepth = false;
bool bIsLabel = false;
switch (pCommand->command)
{
case CCharacterCommand::CC_If:
bIfCondition = true;
bIsIfCondition = true;
++wNestDepth; //indent inside of if block
break;
bUndoOneDepth = true;
break;
case CCharacterCommand::CC_IfElseIf:
bIfCondition = true;
break;
bIsIfCondition = true;
break;
case CCharacterCommand::CC_IfEnd:
if (wNestDepth)
--wNestDepth;
else
wstr += wszExclamation; //superfluous IfEnd
break;
break;
default: break;
}
}

//Unnest If block markers.
switch (pCommand->command)
{
case CCharacterCommand::CC_IfEnd:
case CCharacterCommand::CC_IfElse:
case CCharacterCommand::CC_IfElseIf:
if (wNestDepth)
--wNestDepth;
else
wstr += wszExclamation; //superfluous IfEnd
//no break
case CCharacterCommand::CC_Disappear:
case CCharacterCommand::CC_MoveTo:
case CCharacterCommand::CC_MoveRel:
case CCharacterCommand::CC_EndScript:
case CCharacterCommand::CC_EndScriptOnExit:
case CCharacterCommand::CC_FlushSpeech:
case CCharacterCommand::CC_GoSub:
case CCharacterCommand::CC_GoTo:
case CCharacterCommand::CC_If:
case CCharacterCommand::CC_Imperative:
case CCharacterCommand::CC_Behavior:
case CCharacterCommand::CC_Label:
case CCharacterCommand::CC_LevelEntrance:
case CCharacterCommand::CC_SetMusic:
case CCharacterCommand::CC_Speech:
case CCharacterCommand::CC_TurnIntoMonster:
case CCharacterCommand::CC_PlayerEquipsWeapon:
case CCharacterCommand::CC_SetPlayerStealth:
case CCharacterCommand::CC_SetWaterTraversal:
case CCharacterCommand::CC_StartGlobalScript:
case CCharacterCommand::CC_AnswerOption:
case CCharacterCommand::CC_AmbientSound:
case CCharacterCommand::CC_AmbientSoundAt:
case CCharacterCommand::CC_PlayVideo:
case CCharacterCommand::CC_WorldMapSelect:
case CCharacterCommand::CC_WorldMapMusic:
case CCharacterCommand::CC_WorldMapIcon:
case CCharacterCommand::CC_WorldMapImage:
case CCharacterCommand::CC_ChallengeCompleted:
case CCharacterCommand::CC_Return:
if (bIfCondition)
wstr += wszQuestionMark; //questionable If condition
break;

case CCharacterCommand::CC_ImageOverlay:
if (bIfCondition)
wstr += wszQuestionMark; //questionable If condition
if (pCommand->label.empty()) {
wstr += wszExclamation;
} else {
vector<ImageOverlayCommand> temp;
if (!CImageOverlay::parse(pCommand->label, temp))
//Unnest If block markers.
switch (pCommand->command)
{
case CCharacterCommand::CC_IfElse:
case CCharacterCommand::CC_IfElseIf:
if (wNestDepth)
bUndoOneDepth = true;
else
wstr += wszExclamation; //superfluous IfEnd
//no break
case CCharacterCommand::CC_IfEnd:
case CCharacterCommand::CC_Disappear:
case CCharacterCommand::CC_MoveTo:
case CCharacterCommand::CC_MoveRel:
case CCharacterCommand::CC_EndScript:
case CCharacterCommand::CC_EndScriptOnExit:
case CCharacterCommand::CC_FlushSpeech:
case CCharacterCommand::CC_GoSub:
case CCharacterCommand::CC_GoTo:
case CCharacterCommand::CC_If:
case CCharacterCommand::CC_Imperative:
case CCharacterCommand::CC_Behavior:
case CCharacterCommand::CC_LevelEntrance:
case CCharacterCommand::CC_SetMusic:
case CCharacterCommand::CC_Speech:
case CCharacterCommand::CC_TurnIntoMonster:
case CCharacterCommand::CC_PlayerEquipsWeapon:
case CCharacterCommand::CC_SetPlayerStealth:
case CCharacterCommand::CC_SetWaterTraversal:
case CCharacterCommand::CC_StartGlobalScript:
case CCharacterCommand::CC_AnswerOption:
case CCharacterCommand::CC_AmbientSound:
case CCharacterCommand::CC_AmbientSoundAt:
case CCharacterCommand::CC_PlayVideo:
case CCharacterCommand::CC_WorldMapSelect:
case CCharacterCommand::CC_WorldMapMusic:
case CCharacterCommand::CC_WorldMapIcon:
case CCharacterCommand::CC_WorldMapImage:
case CCharacterCommand::CC_ChallengeCompleted:
case CCharacterCommand::CC_Return:
if (bLastWasIfCondition)
wstr += wszQuestionMark; //questionable If condition
break;
case CCharacterCommand::CC_Label:
bIsLabel = true;
if (bLastWasIfCondition)
wstr += wszQuestionMark; //questionable If condition
break;
case CCharacterCommand::CC_ImageOverlay:
if (bLastWasIfCondition)
wstr += wszQuestionMark; //questionable If condition
if (pCommand->label.empty()) {
wstr += wszExclamation;
}
break;
}
else {
vector<ImageOverlayCommand> temp;
if (!CImageOverlay::parse(pCommand->label, temp))
wstr += wszExclamation;
}
break;

case CCharacterCommand::CC_VarSet:
if (bIfCondition)
wstr += wszQuestionMark; //questionable If condition
//no break
case CCharacterCommand::CC_WaitForVar:
{
//Verify integrity of hold var refs.
switch (pCommand->y)
case CCharacterCommand::CC_VarSet:
if (bLastWasIfCondition)
wstr += wszQuestionMark; //questionable If condition
//no break
case CCharacterCommand::CC_WaitForVar:
{
//Verify integrity of hold var refs.
switch (pCommand->y)
{
case ScriptVars::AppendText:
case ScriptVars::AssignText:
break;
break;
default:
if (!pCommand->label.empty()) //an expression is used as an operand
{
CEditRoomScreen *pEditRoomScreen = DYN_CAST(CEditRoomScreen*, CScreen*,
g_pTheSM->GetScreen(SCR_EditRoom));
CEditRoomScreen *pEditRoomScreen = DYN_CAST(CEditRoomScreen *, CScreen *,
g_pTheSM->GetScreen(SCR_EditRoom));
ASSERT(pEditRoomScreen);
ASSERT(pEditRoomScreen->pHold);
UINT index=0;
UINT index = 0;
if (!CCharacter::IsValidExpression(pCommand->label.c_str(), index, pEditRoomScreen->pHold))
wstr += wszAsterisk; //expression is not valid
}
break;
break;
}
}
break;

//Deprecated commands.
case CCharacterCommand::CC_GotoIf:
case CCharacterCommand::CC_WaitForHalph:
case CCharacterCommand::CC_WaitForNotHalph:
case CCharacterCommand::CC_WaitForMonster:
case CCharacterCommand::CC_WaitForNotMonster:
case CCharacterCommand::CC_WaitForCharacter:
case CCharacterCommand::CC_WaitForNotCharacter:
wstr += wszAsterisk;
break;
default: break;
}
break;

//Deprecated commands.
case CCharacterCommand::CC_GotoIf:
case CCharacterCommand::CC_WaitForHalph:
case CCharacterCommand::CC_WaitForNotHalph:
case CCharacterCommand::CC_WaitForMonster:
case CCharacterCommand::CC_WaitForNotMonster:
case CCharacterCommand::CC_WaitForCharacter:
case CCharacterCommand::CC_WaitForNotCharacter:
wstr += wszAsterisk;
break;
default: break;
}
UINT wFinalIndent = wNestDepth * INDENT_TAB_SIZE;
if (bIsLabel)
wFinalIndent = 0;

if (bIfCondition)
{
wIndent += ifIndent;
if (bIfCondition)
if (wNestDepth) //...but don't include If indentation in the code block
--wNestDepth;
}
else if (bLastWasIfCondition)
wFinalIndent += INDENT_IF_CONDITION_SIZE;

wstr.insert(wstr.end(), wIndent + wNestDepth*tabSize, W_t(' '));
else if (bUndoOneDepth)
wFinalIndent -= INDENT_TAB_SIZE;

return wstr;
wstr.insert(wstr.begin(), bIsLabel ? wIndent - 2 : wIndent, W_t(' '));
wstr.insert(wstr.end(), wFinalIndent, W_t(' '));
wstr += this->pCommandsListBox->GetTextAtLine(index);
this->pCommandsListBox->SetItemTextAtLine(index, wstr.c_str());

bLastWasIfCondition = bIsIfCondition;
}
}

//*****************************************************************************
Expand Down Expand Up @@ -4731,6 +4751,9 @@ void CCharacterDialogWidget::PopulateCommandDescriptions(
GetCommandDesc(commands, pCommand).c_str());
SetCommandColor(pCommandList, insertedIndex, pCommand->command);
}

PrettyPrintCommands(commands);

if (commands.size())
pCommandList->SelectLine(0);
}
Expand Down Expand Up @@ -7747,7 +7770,8 @@ WSTRING CCharacterDialogWidget::toText(
//
//Params:
const COMMANDPTR_VECTOR& commands,
CCharacterCommand* pCommand) //Command to parse
CCharacterCommand* pCommand, //Command to parse
const UINT wCommandIndex) //Index of the command
{
#define concatNum(n) wstr += _itoW(n,temp,10)
#define concatNumWithComma(n) concatNum(n); wstr += wszComma;
Expand All @@ -7761,7 +7785,8 @@ WSTRING CCharacterDialogWidget::toText(
if (wstrCommandName.empty())
return wstr;

wstr += GetPrettyPrinting(commands, pCommand, 6, 3);
UINT indent = ExtractCommandIndent(wCommandIndex);
wstr.insert(wstr.end(), indent - INDENT_PREFIX_SIZE + 2, W_t(' '));
wstr += wstrCommandName;
wstr += wszSpace;

Expand Down
11 changes: 7 additions & 4 deletions DROD/CharacterDialogWidget.h
View file
Open in desktop
Expand Up @@ -79,6 +79,10 @@ friend class CRenameDialogWidget;
bool RenameCharacter();
bool RenameVar();

static const UINT INDENT_PREFIX_SIZE;
static const UINT INDENT_TAB_SIZE;
static const UINT INDENT_IF_CONDITION_SIZE;

private:
void AddCharacterDialog();
void AddCommandDialog();
Expand Down Expand Up @@ -108,9 +112,8 @@ friend class CRenameDialogWidget;
const WCHAR* pText) const;
HoldCharacter* GetCustomCharacter();
WSTRING GetDataName(const UINT dwID) const;
WSTRING GetPrettyPrinting(const COMMANDPTR_VECTOR& commands,
CCharacterCommand* pCommand,
const UINT ifIndent, const UINT tabSize) const;
UINT ExtractCommandIndent(const UINT wCommandIndex) const;
void PrettyPrintCommands(const COMMANDPTR_VECTOR &commands);
WSTRING GetEntranceName(CEditRoomScreen *pEditRoomScreen, UINT entranceID) const;
void AppendGotoDestination(WSTRING& wstr, const COMMANDPTR_VECTOR& commands,
const CCharacterCommand& pCommand) const;
Expand Down Expand Up @@ -168,7 +171,7 @@ friend class CRenameDialogWidget;

//For text editing of script commands.
CCharacterCommand* fromText(WSTRING text);
WSTRING toText(const COMMANDPTR_VECTOR& commands, CCharacterCommand* pCommand);
WSTRING toText(const COMMANDPTR_VECTOR& commands, CCharacterCommand* pCommand, const UINT wCommandIndex);

CListBoxWidget *pGraphicListBox, *pPlayerGraphicListBox, *pAddCommandGraphicListBox;
COptionButtonWidget *pIsVisibleButton;
Expand Down