Skip to content

Marvin console improvements#385

Merged
Try merged 2 commits intoTry:masterfrom
thokkat:marvinMoveCursor
Jan 22, 2023
Merged

Marvin console improvements#385
Try merged 2 commits intoTry:masterfrom
thokkat:marvinMoveCursor

Conversation

@thokkat
Copy link
Copy Markdown
Collaborator

@thokkat thokkat commented Jan 17, 2023

A bit more convenience for marvin console.

The cursor is now moveable, the last input is preserved while moving through history and the delete key is usable.

Comment thread game/ui/consolewidget.cpp Outdated
y-=fnt.pixelSize();
if(i+1==log.size()) {
int x = margins().left + fnt.textSize(log[i]).w;
int x = margins().left + fnt.textSize(log[i].substr(0,cursPos)).w;
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Would this line create(and destroy immediately) new string every frame? One string can't kill performance, but it's generally bad code.

  1. GthFont has overloads with iterators
  2. std::string_view

Comment thread game/ui/consolewidget.cpp Outdated
}
if(e.key==Event::K_Delete) {
if(log.back().size()>cursPos) {
log.back().erase(cursPos,1);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nitpick: tabulation

Comment thread game/ui/consolewidget.cpp Outdated
if(e.key==Event::K_Back) {
if(log.back().size()>0)
log.back().pop_back();
if(log.back().size()>0 && cursPos>0) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Maybe something like
if(0<curssPos && curssPos<=log.back().size())
would be more clean way to express this condition?

@thokkat
Copy link
Copy Markdown
Collaborator Author

thokkat commented Jan 21, 2023

Addressed the changes.

@Try Try merged commit 2e2abcb into Try:master Jan 22, 2023
@Try
Copy link
Copy Markdown
Owner

Try commented Jan 22, 2023

Merged, thanks!

@thokkat thokkat deleted the marvinMoveCursor branch March 21, 2023 17:28
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