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

Add AI and GS to framerate window #7178

Merged
merged 2 commits into from Feb 23, 2019
Merged

Add AI and GS to framerate window #7178

merged 2 commits into from Feb 23, 2019

Conversation

@nielsmh
Copy link
Contributor

@nielsmh nielsmh commented Feb 4, 2019

This can make the framerate window extremely tall, and it should maybe be made scrolling.

@nielsmh nielsmh added the wip label Feb 4, 2019
@PeterN
Copy link
Member

@PeterN PeterN commented Feb 4, 2019

src/framerate_gui.cpp:495:9: warning: unused variable 'linecount' [-Wunused-variable]

@PeterN
Copy link
Member

@PeterN PeterN commented Feb 4, 2019

As these happen in the game loop, and do affect the game loop total monitor, would it be possible to have these within sub-grouping of "scripts" in there, rather than appended to the end?

@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 4, 2019

That's going to be really annoying :P Since the AIs are handled specially right now, with regard to strings.

@PeterN
Copy link
Member

@PeterN PeterN commented Feb 4, 2019

I think we're not meant to use snprintf:

src/safeguards.h:#define snprintf SAFEGUARD_DO_NOT_USE_THIS_METHOD

I think seprintf() is our preferred call, this takes lastof() rather than lengthof()

@LordAro
Copy link
Member

@LordAro LordAro commented Feb 4, 2019

Should probably add safeguards.h in the relevant file.

Played around with it a bit, one suggestion - "AI n - {AI_NAME}" with the inclusion of the dash? I don't think the window is long enough that it's worth bothering about making it scroll

@SamuXarick
Copy link
Contributor

@SamuXarick SamuXarick commented Feb 4, 2019

wormai 2042-11-26
Hi. Can you make it 1-15 instead of 0-14? It would match the console 'companies' and other commands.

@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 5, 2019

Okay I think that's most to-do items? Placed GS and AI under the Game loop heading, and fixed their numbering.
image

This may need testing to make sure everything is cleared/reset properly when the number of active scripts change, e.g. on game load, new game, and such.

@LordAro
Copy link
Member

@LordAro LordAro commented Feb 5, 2019

Would it be worth grouping all the AI/GS together with a "grand total" and having all the individual scripts nested underneath it?

@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 17, 2019

Getting there...
2019-02-17_16-43-49

@nielsmh nielsmh removed the wip label Feb 17, 2019
@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 17, 2019

I think this is everything.

The scripts total is a bit weird, since AIs not running every tick contribute less to the average, so the total can appear to be less than the simple sum.

@michicc
Copy link
Member

@michicc michicc commented Feb 20, 2019

Is this intended to be a squash or do you want to rebase/squash commits yourself?

@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 21, 2019

This was initially intended to be a squash, but maybe it's worthwhile to separate commits for the AI/GS measurements and the resize/scroll behaviour?

PeterN
PeterN previously requested changes Feb 22, 2019
Copy link
Member

@PeterN PeterN left a comment

Clicking to open a graph does not work properly with the scrolling; you need to use this->vscroll->GetScrolledRowFromWidget() instead of this->GetRowFromWidget(). Most windows have a local variable this->vscroll which is assigned in the constructor.

@PeterN PeterN dismissed their stale review Feb 22, 2019

Implemented

@nielsmh
Copy link
Contributor Author

@nielsmh nielsmh commented Feb 23, 2019

Squashed to two logical commits, should be ready for merge now.

@nielsmh nielsmh added this to the 1.9.0 milestone Feb 23, 2019
PeterN
PeterN approved these changes Feb 23, 2019
@nielsmh nielsmh merged commit 13962a8 into OpenTTD:master Feb 23, 2019
8 checks passed
@nielsmh nielsmh deleted the ai-framerate branch Mar 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants