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 instance static class #41

Merged
merged 12 commits into from
Oct 18, 2022
Merged

Add instance static class #41

merged 12 commits into from
Oct 18, 2022

Conversation

Kelwan
Copy link
Contributor

@Kelwan Kelwan commented Oct 4, 2022

Closes #38

Runtime/EcsactDefaults.cs Outdated Show resolved Hide resolved
Runtime/EcsactDefaults.cs Outdated Show resolved Hide resolved
Runtime/EcsactDefaults.cs Outdated Show resolved Hide resolved
Comment on lines 103 to 109
EcsactRunner.OnRuntimeLoad<DefaultFixedRunner>(
EcsactRuntimeDefaultRegistry.RunnerType.FixedUpdate,
"Default Fixed Runner"
);
Copy link
Member

Choose a reason for hiding this comment

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

I think instead of having OnRuntimeLoad set the Default we could have it return the runner in question?

Then this could look like:

Suggested change
EcsactRunner.OnRuntimeLoad<DefaultFixedRunner>(
EcsactRuntimeDefaultRegistry.RunnerType.FixedUpdate,
"Default Fixed Runner"
);
Ecsact.Defaults.Runner = DefaultFixedRunner.OnRuntimeLoad(
EcsactRuntimeDefaultRegistry.RunnerType.FixedUpdate,
"Default Fixed Runner"
);

I'd probably rename OnRuntimeLoad to something more appropriate, however.

Runtime/EcsactRuntime.cs Outdated Show resolved Hide resolved
@zaucy
Copy link
Member

zaucy commented Oct 5, 2022

@Kelwan I updated your PR to point to develop instead of release

@zaucy zaucy changed the base branch from release to develop October 5, 2022 17:08
Runtime/EcsactDefaults.cs Outdated Show resolved Hide resolved
Runtime/EcsactDefaults.cs Outdated Show resolved Hide resolved
@Kelwan Kelwan force-pushed the feat/add-static-instance-class branch 2 times, most recently from f983f50 to 9242d84 Compare October 17, 2022 21:49
@Kelwan Kelwan force-pushed the feat/add-static-instance-class branch from ea0172e to 4bfd4bf Compare October 18, 2022 18:02
@Kelwan Kelwan marked this pull request as ready for review October 18, 2022 18:02
Comment on lines 12 to 16
public static EcsactRuntime Runtime => _Runtime
?? throw new Exception(
"Runtime is null, if you want to access it as early as possible" +
" use Ecsact.Defaults.WhenReady"
);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static EcsactRuntime Runtime => _Runtime
?? throw new Exception(
"Runtime is null, if you want to access it as early as possible" +
" use Ecsact.Defaults.WhenReady"
);
public static EcsactRuntime Runtime => _Runtime ?? throw new Exception(
"Runtime is null, if you want to access it as early as possible" +
" use Ecsact.Defaults.WhenReady"
);

Comment on lines 23 to 25
Ecsact.Defaults.WhenReady(() => {
Load();
});
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Ecsact.Defaults.WhenReady(() => {
Load();
});
Ecsact.Defaults.WhenReady(Load);

@Kelwan Kelwan merged commit 631eca3 into develop Oct 18, 2022
@Kelwan Kelwan deleted the feat/add-static-instance-class branch October 18, 2022 18:37
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.

Static class that encompasses all default instances dictated by settings
2 participants