This repository was archived by the owner on Jan 5, 2024. It is now read-only.
forked from DataRealms/CCOSS
-
Notifications
You must be signed in to change notification settings - Fork 38
Fixing the throttle default values from 4zK's prior work #337
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
7aebffe
Unbroke all the mods :P
garethyr 1883cb2
Garbage changelog entry so it's not forgotten about
garethyr 99a2360
Merge branch 'development' into throttle-fix
garethyr f32bd77
Merge branch 'development' into throttle-fix
garethyr b764b1b
Moved lua bindings into LuaBindingsEntities
garethyr 19fb486
Undid empty line in luaman.cpp
garethyr 69b9c11
Changed throttle logic and renamed `Min/MaxThrottleRange` as `Negativ…
fourZK f02114e
Update CHANGELOG.md
fourZK 015288e
Missed cleanup
fourZK f215efc
Missed cleanup
fourZK File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure you've made this line more complicated than it needs to be.
throttleFactoris set to 1.0F right above, but you're using it in your math so you effectively have1.0F * (1 - std::abs(m_Throttle) + (m_WhateverMultiplier * std::abs(m_Throttle))Also, can you explain the math changes here a bit, cause I'm pretty sure they're wrong.

Before throttle and throttle multipliers both linearly affected the resulting throttle factor (i.e. emission rate) in a totally straightforward way. Bigger throttle and bigger multiplier meant more emission rate, going from 0 (no emissions at min throttle) to 2 (double emissions at max throttle). Yours doesn't abs the multipliers, meaning that their signs matter (though reasonably they should always be 0 - 1 so that's not a big deal, but probably needs enforcement in the setters), and when I plug the numbers into excel your stuff is super weird (unless I've made a mistake):
If you wanna do it yourself, the formulas are:
Old -
=1 + (A2*ABS(B2))New -
= 1 * (1-ABS(A2)) + (B2*ABS(A2))There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave the formulas to Trez in beforehand to be automatically handled by the converter. I guess it's a thing worth putting into the changelog entry even though it'll probably end up confusing modders, especially when this stuff hasn't ever been very apparent in mods.
I thought we discussed the throttle logic change already so I don't really know what you're frothing about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol it's 100% possible that we did, but also possible that I've forgotten since it was a while back, or that I didn't fully understand.
Still, this seems straight up incorrect to me, like why would having throttle 0.1 and positive throttle multiplier 0.1 result in 0.9 strength emissions? Feel free to shoot me a discord message if you'd rather discuss there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After discussion with 4zK, we're keeping this as-is. There's a followup issue to get rid of these confusing code concepts - #374.