Skip to content

fix: cables duplication #5961

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

Merged
merged 5 commits into from
Jan 24, 2025
Merged

Conversation

Goredell
Copy link
Contributor

@Goredell Goredell commented Jan 24, 2025

Why should this PR be merged?

What does this PR do?

Fixes cables not being removed after linking vehicles, by revisiting some of the logic changes from #4968, without disturbing the work done

Steps to test and verify this PR

"Using" things that use charges and should or should not "disappear" after.

Checklist

Mandatory

@github-actions github-actions bot added the src changes related to source code. label Jan 24, 2025
@Goredell Goredell marked this pull request as ready for review January 24, 2025 01:29
@Goredell
Copy link
Contributor Author

IMO this is still done in an incredibly ugly way, items with a special way of handling charges and their dis//re-appearance - such as cables, should be handled somehow more explicit

@scarf005
Copy link
Member

image
image

I've fixed the link but please read the checklist before checking next time.

@Goredell
Copy link
Contributor Author

Goredell commented Jan 24, 2025

I've fixed the link but please read the checklist before checking next time.

Thanks, boss, sorry, boss

@scarf005 scarf005 requested a review from chaosvolt January 24, 2025 02:23
@scarf005
Copy link
Member

I've fixed the link but please read the checklist before checking next time.

Thanks, boss, sorry, boss

no worries, should've added fixes in PR template anyways

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

  1. Compiled and load-tested.
  2. Spawned in a pair of jumper cables, and used them plus a shopping cart to bootleg a connection across two tiles, no duplicate jumper cables left in inventory.
  3. Spawned in a battery in the area now linked to the evac shelter, confirmed that examining the battery tells you about the grid it's bootleg-linked to, and confirmed after enough waiting that it will charge eventually.
  4. Spawned in a compound bow, cycling through different modes works as expected.
  5. Spawned in and lit a flare, it turned into a lit flare then a dead flare on running out, as expected.
  6. Spawned in a flashlight, turning on and off works as expected, and behaves as intended on running out of charges.

Bug: Spawned in Rising Sun, activated it, and let it run out of charges. It vanishes, this PR reintroduces #5868. Note that while fire weapons are currently obsoleted in mainline, they are still in use in in-repo mods and bringing them back into mainline is still on the table for things we're thinking about doing someday, plus the function finds use for items in other mods out there.

image

@github-actions github-actions bot added docs PRs releated to docs page JSON related to game datas in JSON format. labels Jan 24, 2025
Copy link
Contributor

autofix-ci bot commented Jan 24, 2025

Autofix has formatted code style violation in this PR.

I edit commits locally (e.g: git, github desktop) and want to keep autofix
  1. Run git pull. this will merge the automated commit into your local copy of the PR branch.
  2. Continue working.
I do not want the automated commit
  1. Format your code locally, then commit it.
  2. Run git push --force to force push your branch. This will overwrite the automated commit on remote with your local one.
  3. Continue working.

If you don't do this, your following commits will be based on the old commit, and cause MERGE CONFLICT.

@Goredell
Copy link
Contributor Author

So, since previous change introduced a bug with vanishing items, I made a decision to handle those in explicit way, to remove a possibility of unexpected item vanishing.
Also found a new bug, throwable fire extinguishers don't actually pop, when thrown on fire. Don't want to fix it here, gonan fill out a proper issue.

@Goredell Goredell marked this pull request as draft January 24, 2025 07:47
@Goredell Goredell marked this pull request as ready for review January 24, 2025 12:29
@Goredell Goredell requested a review from chaosvolt January 24, 2025 12:29
@chaosvolt
Copy link
Member

Oh fuck me. So I've got good news and bad news.

Good news is, it's not your PR that did this. I just tested in 2025-01-22 and it still happens. Bad news is the bug either has returned from beyond the grave after #5870 was merged or that PR was merged without testing (can't tell, but PR OP indicates I was asked to test as Kheir was busy and I didn't get to it in time before it got merged)

Copy link
Member

@chaosvolt chaosvolt left a comment

Choose a reason for hiding this comment

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

Apologies for the false positive on the bug, since everything else works fine and I confirmed the fire weapon bug is present in one of the most recent releases, this is good to go.

If you want to go back to the other implementation instead that'd be fine, though having an item flag for tools that vanish when used up is actually something that's been on my "to look into someday" list since it'll be useful for other stuff in the future, so I kinda really like this implementation too.

EDIT: Went ahead with the merge since honestly the derailment due to finding an unrelated bug has kept you waiting long enough already, I screm

@chaosvolt chaosvolt merged commit 044e30d into cataclysmbnteam:main Jan 24, 2025
17 checks passed
@scarf005
Copy link
Member

Also found a new bug, throwable fire extinguishers don't actually pop, when thrown on fire. Don't want to fix it here, gonan fill out a proper issue.

that would be #3723

@Goredell Goredell deleted the fix-cable-dublication branch March 13, 2025 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs PRs releated to docs page JSON related to game datas in JSON format. src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicating Jumper Cables
3 participants