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

Fix: Allow tools with charged batteries to be modded #77101

Closed
wants to merge 3 commits into from

Conversation

Pabblusansky
Copy link

Summary

None

Purpose of change

Simply makes it possible to attach mods to tools. fixes #76717

Testing

Compiled the game and tested - attaching the mod to a tool works perfectly.

Simply makes it possible to attach mods to tools. fixes CleverRaven#76717
@github-actions github-actions bot added new contributor <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` labels Oct 17, 2024
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@github-actions github-actions bot added astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels Oct 17, 2024
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Oct 22, 2024
@Maleclypse
Copy link
Member

Hey so I'm trying to understand why someone would do this when the e-ink has { "type": "link_up", "cable_length": 4, "charge_rate": "10 W" }

@Maleclypse
Copy link
Member

Hey so I'm trying to understand why someone would do this when the e-ink has { "type": "link_up", "cable_length": 4, "charge_rate": "10 W" }

There may be a good reason. And if so I’m open to it.

@Pabblusansky
Copy link
Author

Hey so I'm trying to understand why someone would do this when the e-ink has { "type": "link_up", "cable_length": 4, "charge_rate": "10 W" }

Hi. So the PR is not about the charge_rate of the e-ink PC, it's about the impossibility of attaching a tool (power conversion, battery, etc) to a charged device. Please see the original issue: #76717 for more context.

@GuardianDll
Copy link
Member

Do you happen to know how easy it would be to disable link_up use action if toolmod is attached? bugfix is desired, but i am afraid it would allow to use both batteries and link_up action, which is busted and probably not very desired

@Maleclypse Maleclypse changed the title Fix "Can't attach power conversion mod to e-ink tablet PC" Fix: Allow tools with charged batteries to be modded Oct 26, 2024
@Maleclypse
Copy link
Member

Please review the name change above. I renamed this PR to make it more accurate since it's scope is larger than e-ink items. This seems like a change that's going to cause other downstream bugs potentially so I'll have to leave this decision to other mergers with stronger code backgrounds.

@Pabblusansky
Copy link
Author

Do you happen to know how easy it would be to disable link_up use action if toolmod is attached? bugfix is desired, but i am afraid it would allow to use both batteries and link_up action, which is busted and probably not very desired

Oh, I'm sorry. A bit confused, now I understand the point of the question. I'll try to see if I can do something about this potential “exploit”, but if it doesn't work, I think I'd listen to the opinions of other contributors.

@kevingranade
Copy link
Member

It's not so much an exploit as it is "a very high risk of introducing new bugs".
The exemplar of the e-ink tablet in particular is pretty much invalid anyway, it already has a battery and a charger, that's all it's supposed to have and adding a battery pack isn't really a sensical thing to do.

@Pabblusansky Pabblusansky marked this pull request as draft October 28, 2024 20:07
@Pabblusansky
Copy link
Author

Okay, closing the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions new contributor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't attach power conversion mod to e-ink tablet PC
4 participants