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

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 Work in progress. Feature branch that will require feedback during the development process label Feb 4, 2019
@PeterN
Copy link
Member

PeterN commented Feb 4, 2019

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

@PeterN
Copy link
Member

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 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 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 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

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 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 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 commented Feb 17, 2019

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

@nielsmh nielsmh removed the wip Work in progress. Feature branch that will require feedback during the development process label Feb 17, 2019
@nielsmh
Copy link
Contributor Author

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 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 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

Choose a reason for hiding this comment

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

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.

@nielsmh
Copy link
Contributor Author

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
@nielsmh nielsmh merged commit 13962a8 into OpenTTD:master Feb 23, 2019
@nielsmh nielsmh deleted the ai-framerate branch March 8, 2019 18:45
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.

None yet

5 participants