Best way to implement "reverse battery compartment mod" within current code structure?

First, don’t imitate and mock me. You want help, provide enough information in the first place to get them.

Second, no. You’ve answered a lot of other questions in between and in that very post I’ve referenced you were talking about the point of definition of BATTERY_MOUNT and the “location”: “on_battery_mount”, not that particular line that was mentioned 9 posts above.
I don’t see what file you’re currently looking at, and I (thankfully) can’t read your mind, so you need to provide these information directly.

Third, even though you’ve really pissed me off by this point, the answer to your question is 11 lines above where my link points:

// if a windshield is removed (usually destroyed) also remove curtains
// attached to it.

So, translated onto that specific line, the use would be: "If the battery holder gets destroyed, it also removes the battery attached to it."

Then how about you don’t start off with the incredibly patronizing assumptions that I legitimately had no idea what all of C++ code is for, not just one specific line?

That’s much more helpful, thank you. Will try and test if that C++ line is still required for this interaction if .json flags have been properly defined.

Once again: I don’t know what you’re thinking, I don’t know your level of knowledge, and I have to go by what you asking. I’ve answered your question as accurate as possible.
I have no idea how you think that answering a question directly is “patonizing”, but if you prefer me to not answer you at all, I can do that…

If you want to continue playing this game, sure:
do notice how I wasn’t really asking anything. I just made an off-hand comment how I don’t understand the exact role of a segment of C++ code. It was you who decided to interpret that as an actual question and “answer” it.

It very much is, when there wasn’t really a question to begin with, and you started your “answer” with the assumption I have no idea what any of C++ code does.

Anyway, I do apologize for any miscommunication on my part.

I don’t play any games with you. But I will continue this discussion in private messaging to not derail this topic.

1 Like

I tested this theory.
What I did was merge the .json code, but NOT the C++ code.

remove_dependent_part( "HANDHELD_BATTERY_MOUNT", "NEEDS_HANDHELD_BATTERY_MOUNT" );

Then I installed the new tool battery mount that works via HANDHELD_BATTERY_MOUNT flag + a battery that works via NEEDS_HANDHELD_BATTERY_MOUNT flag.
And then I smashed it. Here is the result:

As you can see, destruction with damage still detaches the dependent part even without the C++ code.

As such, I’m still not sure if this C++ line:

remove_dependent_part( "BATTERY_MOUNT", "NEEDS_BATTERY_MOUNT" )

is actually anything but obsolete.

Created a PR for it, all feedback would be welcome:

I tested this a bit and came to the conclusion that I have to disagree with you.

When I’ve started testing I did not have your pull request, so I’ve worked with the things that were already there, namely: remove_dependent_part( "SEAT", "SEATBELT" ) and remove_dependent_part( "WINDOW", "CURTAIN" ).

I’ve commented out one of the lines (seat/seatbelt) and left the other one active (window/curtain).
Smashing the tile did actually drop it, yes.
But giving my character the mutation Debug Hammerspace and then tried to remove the seat, it removed it and left the seatbelt in the vehicle. Doing the same with a window with a curtain removed the window (dropping the sheet of glass to my feet) and the curtain (leaving it as a sheet item on the tile it was installed).

Undoing the code change and trying it with the seat/seatbelt again gave the same result as the window/curtain, proving that there are actually still cases that make use of that part of the code.

I had a 14 hour work shift and little sleep before and after, and I’m too exhausted to try to hunt down more situations in which this can occure or to test if your pull request behaves in the same way; so I would appreciate if you could run that test on your code yourself.

Trying to remove my new part (tool battery mount) with a battery attached to it gives me this:

And that’s WITHOUT the C++ line added to the game.

Yes. But hitting enter on it will still remove the part, leaving the attached part on the vehicle (as from the experience I had with the seat/seatbelt situation).

Hitting enter my hardest, nothing happens. ¯\_(ツ)_/¯
(If I move 1 position down and hit enter on the battery, it’s removed right away.)

…while having the mutation Debug Hammerspace active (just making sure)?
Because as stated, you can remove parts that have others attached to it with the Hammerspace mutation… It just drops the attached part in place.
Not having the mutation usually results in no movement at all.

Ah, yes, with the mutation active I can remove the mount without removing the battery first.
And it leaves the battery atteched without the C++ line, and removes it with the C++ line.

However, this brings another question: what non-debug scenario is this type of removal equivalent to?
I don’t mind having a similar C++ line for my parts just-in-case, I just wonder if there still is a “natural” scenario for it to become relevant.

Usually I’d go code diving to find more cases and also track down in which cases the debug mutation differentiates from other remove cases (and if some remove cases can go that path too, resulting in the same behavior). I’ve mentioned above why I didn’t do that this time.

For now all I’ve wondered was if it is actually obsolete or not.

Understandable.
Thank you.

I’ve added an equivalent C++ line for my parts anyway, so it’s more of a theoretical question at this point…

While we are at it: is there an online tool for C++ code linting?
Github complains about linting despite my line being formatted seemingly exactly the same as the line above it…

From what I see in your pull request, it complains about the vehicle_parts.json file, so it’s not the C++ part you have to worry about.
As for your question: Based on JSON_STYLE.md it uses a custom formatting (“We haven’t been able to find a decent JSON styling tool, so we wrote our own.”) and tools are provided in the releases ("json_formatter.exe or json_formatter.cgi"), as well as online at Cataclysm: DDA Web JSON Linting Tool .

1 Like

Yea, you are right.
For some reason I thought there was also a linting error for C++, but I guess I just imagined it.

we use astyle for c++ linting, and yes PRs will fail tests if they have c++ code that violates expected formatting.

please see doc/CODE_STYLE.md for more details.

(if you had some c++ code at some point then removed it, it’s possible for tests to catch that and complain, if the tests completed before you removed it)

Any ideas what’s up with merging?
Been a solid week… Anything I need to do to speed it up?