Skip to content

Stop warp only if there are runnable expierments#16

Open
canisin wants to merge 2 commits intothewebbooth:masterfrom
canisin:patch-1
Open

Stop warp only if there are runnable expierments#16
canisin wants to merge 2 commits intothewebbooth:masterfrom
canisin:patch-1

Conversation

@canisin
Copy link
Copy Markdown

@canisin canisin commented Jun 21, 2017

As per discussion on the forums.

if( _filter.TotalCount > 0 )
{
if( _filter.TotalCount - _filter.CompleteCount > 0 )
if( _filter.DisplayScienceInstances.Any(x => CanRunExperiment(x)) )
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we find a non-LINQ way to do this test? LINQ is great for code readability, but it creates a lot of temporary objects that have to be garbage collected, and that's not good for consistent game performance.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

well, any linq can be converted to a basic loop more or less easily. but i am not sure if it would save us much in terms of performance. premature optimization is the root of all evil and all that :) but let me try and create a new pull request that gets rid of the linq and then you can decide to adopt either.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have created a new commit that replaces linq with a regular loop. I could not create a pull request this time. I just don't really know how it works. I hope the new commit is visible to you. If not, contact me on the forums.

@mwerle
Copy link
Copy Markdown

mwerle commented Aug 1, 2017

"without linq" - good, but in general you want to avoid "foreach" as well if you're already going to this level of optimization, unless you know for sure that the foreach won't generate garbage.

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.

3 participants