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

Componentizes rechargers & unwrenching #5107

Closed
wants to merge 15 commits into from

Conversation

Zonespace27
Copy link
Contributor

@Zonespace27 Zonespace27 commented Dec 2, 2023

About the pull request

Converts rechargers into a component.
Also converts the ability to unwrench things into an element due to it being needed to have feature parity w/ recharger componentization.

Marked as draft until I add cell components to everything that can be put into a recharger.

Explain why it's good for the game

I want to be able to add a form of recharger to more things, so componentization is necessary.

🆑
fix: You can no longer unwrench certain atmos parts.
\🆑

@Zonespace27 Zonespace27 added the Code Improvement Make the code longer label Dec 2, 2023
Copy link
Member

@fira fira left a comment

Choose a reason for hiding this comment

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

Looks good to me overall

code/datums/components/recharger.dm Show resolved Hide resolved
code/datums/elements/simple_unwrench.dm Outdated Show resolved Hide resolved
@cm13-github cm13-github added the Merge Conflict PR can't be merged because it touched too much code label Dec 6, 2023
@cm13-github
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Zonespace27 Zonespace27 marked this pull request as ready for review December 13, 2023 03:09
@cm13-github cm13-github removed the Merge Conflict PR can't be merged because it touched too much code label Dec 13, 2023
@cm13-github
Copy link
Contributor

Conflicts have been resolved. A maintainer will review the pull request shortly.

Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

@Drulikar Drulikar marked this pull request as draft December 14, 2023 11:35
code/_onclick/human.dm Show resolved Hide resolved
code/datums/components/cell.dm Outdated Show resolved Hide resolved
code/datums/components/recharger.dm Show resolved Hide resolved
code/datums/components/recharger.dm Outdated Show resolved Hide resolved
code/datums/components/recharger.dm Outdated Show resolved Hide resolved
Copy link
Contributor

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Dec 23, 2023
@Zonespace27 Zonespace27 marked this pull request as ready for review December 24, 2023 23:44
@Drulikar Drulikar removed the Stale beg a maintainer to review your PR label Dec 30, 2023
Copy link
Contributor

@Drulikar Drulikar left a comment

Choose a reason for hiding this comment

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

I don't know all the possible things that are in game that might be recharged but I can confirm defibs and smart gun batteries recharge fine. However, it doesn't seem like you can recharge M2 Nightvision goggles using a cell (specifically a /obj/item/cell/hyper) because that is still accessing the .charge of the cell which in this case is -1 to indicate it should spawn w/ full charge. So there are still lingering references to charge that aren't using the cell component.

I'd recommend deleting that charge var; look at where all the compiler errors, ensure you properly get the charge from the component where needed, then return the var back if its still needed.

image

@Drulikar Drulikar marked this pull request as draft December 30, 2023 06:06
@Drulikar Drulikar added the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Dec 30, 2023
Copy link
Contributor

github-actions bot commented Jan 7, 2024

This PR has been inactive for long enough to be automatically marked as stale. This means it is at risk of being auto closed in ~ 7 days, please address any outstanding review items and ensure your PR is finished, if these are all true and you are auto-staled anyway, you need to actively ask maintainers if your PR will be merged. Once you have done any of the previous actions then you should request a maintainer remove the stale label on your PR, to reset the stale timer. If you feel no maintainer will respond in that time, you may wish to close this PR youself, while you seek maintainer comment, as you will then be able to reopen the PR yourself

@github-actions github-actions bot added the Stale beg a maintainer to review your PR label Jan 7, 2024
@Zonespace27 Zonespace27 removed the Testmerge Candidate we'll test this while you're asleep and the server has 10 players label Jan 13, 2024
@Zonespace27
Copy link
Contributor Author

Will finish eventually

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Improvement Make the code longer Stale beg a maintainer to review your PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants