Combining AsyncLoadWrapper and DelayedLoadWrapper into LoadWrapper #1181
Conversation
…e old AsyncLoadWrapper and DelayedLoadWrapper classes. Syntactically mostly the same: Only if you used a DelayedLoadWrapper instance without explicitly setting a TimeBeforeLoad which used to set a default delay of 500 (ms), this will now set it to zero in the new constructor, making it not delayed. This is so that all "AsyncLoadWrapper" constructors will still work. If you want to delay the loading, you will now HAVE to explicitly change the TimeBeforeLoad. Every DelayedLoadWrapper implementation already explicitly defined it, so nothing really had to be changed (except for obviously the naming).
…lasses (use LoadWrapper as the alternative)
…edLoadWrapper.cs files.
…f DelayedLoadWrapper.
|
Tempted to keep the remaining class called DelayedLoadWrapper, and possible passing in the time before load as a ctor parameter. @Tom94 @smoogipoo thoughts on this? |
|
Would keep the AsyncLoadWrapper naming, but keep it as it is otherwise. |
|
@FreezyLemon let's go with keeping |
|
Should I change default behaviour then? I mean, if it's called DelayedLoadWrapper, shouldn't it delay by default? (Atm it works like the old AsyncLoadWrapper used to if you don't change the TimeBeforeLoad after constructor call) |
|
i’d put the delay in the ctor |
…nymore. (Implementation changes too)
…ntent evaluation statement.
…" bug" This reverts commit 539c146.
| /// <remarks>If <see cref="timeBeforeLoad"/> remains unchanged (at 0), the loading process will not be delayed.</remarks> | ||
| /// <param name="content">The <see cref="Drawable"/> to be loaded.</param> | ||
| /// <param name="timeBeforeLoad">The delay in milliseconds before loading begins. Zero means immediate loading.</param> | ||
| public DelayedLoadWrapper(Drawable content, double timeBeforeLoad = 0) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| } | ||
| }), | ||
| }, | ||
| 500), |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| Size = new Vector2(0.25f) | ||
| }), | ||
| }, | ||
| 500), |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
| [BackgroundDependencyLoader] | ||
| private void load() | ||
| { | ||
| if (ShouldLoadContent) |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
…u-framework into load-optimization
… newlines (as requested)
…sync call by inline code and changed the doc comments to be a bit more accurate.
| /// In order to benefit from delayed load, we must be inside a <see cref="ScrollContainer"/>. | ||
| /// </summary> | ||
| public class DelayedLoadWrapper : AsyncLoadWrapper | ||
| public class DelayedLoadWrapper : Container |
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
3 participants
What the title says.
Also updated the Tests to implement the new class (works on my end).
Some changes will have to be made in the main game repository to work with this (namely renaming all previous wrapper class/constructor uses to "LoadWrapper" instead. Yes, that's it).
This is referring to this issue over at the main repo. I will also post a PR there including this change alongside the other requested changes.