Skip to content
This repository was archived by the owner on Feb 17, 2026. It is now read-only.

Implement simple batteries#483

Merged
Indy2222 merged 15 commits intoDigitalExtinction:mainfrom
JackCrumpLeys:feat/energy/battery
Jun 17, 2023
Merged

Implement simple batteries#483
Indy2222 merged 15 commits intoDigitalExtinction:mainfrom
JackCrumpLeys:feat/energy/battery

Conversation

@JackCrumpLeys
Copy link
Copy Markdown
Contributor

@JackCrumpLeys JackCrumpLeys commented Jun 9, 2023 •
edited by Indy2222
Loading

This implements the first step towards energy (#473).

@JackCrumpLeys JackCrumpLeys marked this pull request as draft June 9, 2023 07:53
Indy2222
Indy2222 suggested changes Jun 9, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Indy2222 Indy2222 left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

I had a bunch of comments but the overall structure is good.

Comment thread crates/energy/Cargo.toml Outdated
Comment thread crates/spawner/src/spawner.rs Outdated
Comment thread crates/spawner/src/spawner.rs Outdated
Comment thread crates/energy/src/lib.rs Outdated
Comment thread crates/energy/src/battery/systems.rs Outdated
Comment thread crates/energy/src/battery/component.rs Outdated
Comment thread crates/energy/src/battery/component.rs Outdated
Comment thread crates/energy/src/battery/component.rs Outdated
Comment thread crates/energy/src/battery/systems.rs Outdated
Comment thread crates/energy/src/battery/component.rs Outdated
Indy2222
Indy2222 suggested changes Jun 12, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Indy2222 Indy2222 left a comment

Choose a reason for hiding this comment

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

Even though the PR is still a draft, I left a few comments. They mostly regard the style / taste in which things are implemented.

If you feel that such things are not well documented or communicated, please either let me know or open a PR with improvements to e.g. CONTRIBUTING.md. Thank you!

Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/spawner/src/spawner.rs Outdated
Indy2222
Indy2222 suggested changes Jun 15, 2023
View reviewed changes
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
@JackCrumpLeys JackCrumpLeys requested a review from Indy2222 June 16, 2023 07:42
@JackCrumpLeys
Copy link
Copy Markdown
Contributor Author

JackCrumpLeys commented Jun 16, 2023

In adding the UI I am not sure it I did it correctly / using best methods. I will move out of draft now. (I tried really hard to get a screenshot but on Ubuntu some issues with full screen disallow me to (I should open an issue for that))

@JackCrumpLeys JackCrumpLeys marked this pull request as ready for review June 16, 2023 07:49
Indy2222
Indy2222 suggested changes Jun 16, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Indy2222 Indy2222 left a comment

Choose a reason for hiding this comment

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

Could you separate the HUD modifications to another PR?

Otherwise, I had only two minor comments which we can quickly fix and merge it.

Comment thread crates/energy/src/battery.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
@Indy2222 Indy2222 changed the title Battery system basics Implement simple batteries Jun 16, 2023
@JackCrumpLeys JackCrumpLeys force-pushed the feat/energy/battery branch 2 times, most recently from 98ed98b to 4be94fc Compare June 17, 2023 03:57
@JackCrumpLeys JackCrumpLeys requested a review from Indy2222 June 17, 2023 04:07
Indy2222
Indy2222 suggested changes Jun 17, 2023
View reviewed changes
Copy link
Copy Markdown
Collaborator

@Indy2222 Indy2222 left a comment

Choose a reason for hiding this comment

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

Two nitpicks and we are good to go.

Comment thread crates/controller/src/hud/details.rs Outdated
Comment thread crates/energy/src/battery.rs Outdated
@JackCrumpLeys JackCrumpLeys requested a review from Indy2222 June 17, 2023 21:22
Indy2222
Indy2222 previously approved these changes Jun 17, 2023
View reviewed changes
@Indy2222 Indy2222 added this pull request to the merge queue Jun 17, 2023
Merged via the queue into DigitalExtinction:main with commit 99b90b2 Jun 17, 2023
@JackCrumpLeys JackCrumpLeys mentioned this pull request Jun 18, 2023
Merged
@JackCrumpLeys JackCrumpLeys mentioned this pull request Jun 25, 2023
Open
4 tasks
github-merge-queue Bot pushed a commit that referenced this pull request Jun 26, 2023
@JackCrumpLeys
Display energy & num selected in HUD (#520)
69feea5 
Implements part of #397. Adds on to #483 and works towards #473.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@Indy2222 Indy2222 Indy2222 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JackCrumpLeys @Indy2222