Pine bough harvest changes

[chaosvolt]

The end result of a discussion had in private on the forums, regarding a possible solution to some things relating to how many trees have to be stripped down when acquiring pine boughs.

And for added Fun, pushing this now because I tried to compile-test and it instead error'd, citing errors unrelated to my changes, instead involving what I'm guessing was the recent work on eating code, judging by the files cited.

• Amount of pine boughs harvested changed from a flat 2 to randomly 2-4. My original change was a mistake caused by mis-reading the whole function, so the original "same volume in pine boughs" reasoning is no longer a valid excuse. ;w;

[Coolthulhu]

Secondly I've noticed that the actual results seem to always be just 2 pine boughs fairly consistently, when it claims to be 1-4.

1-4 cones, 2 boughs.

[chaosvolt]

Oh wait. I see some fail on my part.

     m->spawn_item(p->pos(), "pine_bough", 2, 12 );
-    m->spawn_item( p->pos(), "pinecone", rng( 1, 4 ) );
+    m->spawn_item( p->pos(), "pinecone", 1, 6 );

...I changed the amount of PINECONES spawned. >.<

[chaosvolt]

Hmm. That does make it harder to justify increasing the number of pine boughs if it already has equal volume to the max number of pinecones dropped.

Maybe it could still be changed to return RNG 2-4 though?

EDIT: Speaking of which...if that's the case, what DOES the 12 in that line mean then?

[Coolthulhu]

Speaking of which...if that's the case, what DOES the 12 in that line mean then?

Not sure what was the original intent - might even have been rng(2, 12). Actual result is that it supplies charges for an item that has no charges and so that 12 is ignored.

[chaosvolt]

Ah, I see. Hmm. 2-4 is at least a lot more restrained than 2-12. Then again...either way, if the end result is a dead pine tree, I'm wondering what a plausible maximum would be.

[macrosblackd]

So, looking through the logs, it used to be rng(2, 12)
Might I suggest 2,6 or 2,8 ? There are a number of recipes that use pine boughs that I'm fairly certain most players don't bother with because it's more tedious than viable alternatives.

[macrosblackd]

In particular, it was buffed from rng( 1, 8 ) when wilderness shelters were added as a craftable thing.

[chaosvolt]

Ah, very peculiar. Could be as high as 2-6 or 2-8, yeah. If anything, the broken RNG reference means it's been nerfed from the older 1-8. >.o

[macrosblackd]

It was buffed to 2-12 in the same day, so more likely 1-8 wasn't giving the numbers that @Rivet-the-Zombie wanted. Honestly, I say put it back to 2-12 and if it seems like too much, nerf it then.

[chaosvolt]

Hmm. Guess I can always push it to that number and dial it back if that proves to be overkill, ja.

[Skidborg]

Given the number of logs and 2x4s that can be taken from a single tree, increasing the number of boughs from a pine tree seems extremely reasonable.

[kevingranade]

And for added Fun, pushing this now because I tried to compile-test and
it instead error'd

GET. IT. COMPILING.
I don't care how much trouble you're having getting it to compile, if you
aren't compiling and testing your changes, you aren't qualified to
contribute.

[macrosblackd]

Chaos, if you're using Codeblocks, a fix has already been merged, if you're using CMake, you need to add Consumption.cpp to src/CMakeFiles.txt.

[chaosvolt]

It was due to the Consumption.cpp, and as it's Codeblocks I'm using, I'm hoping there are no further problems?

So nice to have Kevin griping at me when the compile error was not my fuckup this time. If you haven't figured it out yet, I've BEEN doing the fucking compile tests. The fact that the errors cited had absolutely nothing to do with the changes I made was why I committed it anyway, as it made me suspect it was due to the addition of new makefile content.

[Coolthulhu]

you need to add Consumption.cpp to src/CMakeFiles.txt.

Crap, I knew I forgot something

[chaosvolt]

Yay. -_-

[illi-kun]

@chaosvolt sorry but Kevin is right: how you will distinct the difference between two conditions: (1) build is broken initially, and (2) build is broken initially and due to your changes? For confirmation the fact your PR doesn't brake anything you have to start from working version. So, you have two options: 1) fix the reason it initially broken or 2) wait for someone who will fix it in master.

[illi-kun]

Upd: BTW, it's not about this PR, it's more about general case.

[Coolthulhu]

Adding [WiP] for untested PRs would be enough, IMO.
WiP implies "do not mergetest, it doesn't work yet".

[chaosvolt]

If I had no concrete reason to suspect that the problem was unrelated to my changes, I wouldn't have been confident in committing them anyway. In the general case yes, this makes sense. Especially since I'm the goddamn least competent person when it comes to understanding the source code, I can't always tell what I fucked up in the process.

In this specific case, I committed the changes anyway due to having reasonable suspicion that the issue was unrelated to my changes.

[kevingranade]

If I'd had some indication that you had things compiling in general previous to this PR and the issue just came up, I wouldn't have said anything, but I had no such indication.

Yay. -_-

Also, stop it with the ASCII reaction faces, they don't contribute anything.

[chaosvolt]

If I'd had some indication that you had things compiling in general previous to this PR and the issue just came up, I wouldn't have said anything, but I had no such indication.

See opening post in #15113, discussion about half a dozen posts down in #15085, and all the way back to discussion of my first successful compiles in #14959.

[kevingranade]

This latest temper tantrum is the last straw, I've given you dozens of chances to improve your shitty attitude, and it just hasn't happened, I'm not going to work with you any more and you're no longer welcome in my project.