Add Tribe of Tjernobyl mode #367
Conversation
There was a problem hiding this comment.
No need to have separate commits for bugs and typos and iterations introduced within a PR that adds new functionality, makes it harder to review individual commits as suddenly what you just read and pushed on your mental stack got removed or changed rather than built upon.
Ideally a PR is a nice little story, "Here's new toy A, and here comes new toy B, and here A and B create wonders together".
The health/weapon/breakondeath/togglequad settings are mode agnostic and would be nice to have as a separate commit before introducing the ToT feature that builds on these. If there are any questions from the other guys on ToT, those generic bot settings can easily be merged before ToT to trim down the ToT PR size a bit.
Otherwise it looks pretty harmless. Risk of conflicting a bit with the CTF bot support PR, but will probably be mergable before, so heads up to @SpookyScaryDev
Oh, and no need to close PR #362 and open this new PR, you can just forcepush to the same branch to keep the comment trail referrable. If it got closed due to deleting the branch, pushing to a branch with the same name will also reopen the PR, but lets stick with this new PR this time.
| @@ -2108,6 +2133,94 @@ static void FrogbotsDisable(void) | |||
| } | |||
| } | |||
|
|
|||
| static void FrogbotsSetHealth(void) | |||
There was a problem hiding this comment.
These functions introduce mode agnostic bot customizations unrelated to ToT. Would be nice to have their implementations and usages in a separate commit. This could even be a separate PR that's merged swiftly as they're isolated and clean, no need for separate PR if there are no questions about ToT though.
| @@ -1044,6 +1044,9 @@ void FirstFrame(void) | |||
| RegisterCvarEx(FB_CVAR_DEBUG, "0"); | |||
| RegisterCvarEx(FB_CVAR_ADMIN_ONLY, "0"); | |||
| RegisterCvarEx(FB_CVAR_FREEZE_PREWAR, "0"); | |||
| RegisterCvarEx(FB_CVAR_HEALTH, "100"); | |||
| RegisterCvarEx(FB_CVAR_WEAPON, "2"); | |||
| RegisterCvarEx(FB_CVAR_BREAK_ON_DEATH, "1"); | |||
I generally disagree with this philosophy: the commits themselves tell the story, not the aggregation of code lumped together. That's what the "files changed" tab is for. When a particular development doesn't work out or needs tweaking, it's much simpler to refer to the specific commit in question. For example, slime had other commits that aren't included in this PR because they address code or features that don't exist yet in the repo. My ability to exclude them in this PR would've been hampered considerably had he bundled his commits together. In the story of this project's development, it's important to memorialize these changes not only so we can take notice, but also to easily revert, amend or exclude them without having to modify code directly. |
|
The value is that it makes the commits easier to review. There's no need to encode mistakes into history. But even if you would want to have all the mistakes along the way saved for the future to judge, it would still be valuable to separate out generic functionality hiding in larger code changes. The "files changed" presents the bad view without a story, just a heap of changes without relevant commit messages. Hope it was clear that I wasn't suggesting squashing all commits to a single one. But anyways, that's just my take, so comments were for you to make it easier for whoever will review this when the others get time. |
|
Thanks @dusty-qw for the contribution! |
Okay here's the third attempt to merge ToT mode. I ripped out the skill for ping feature, so this no longer depends on any MVDSV changes.
ToT is a bot FFA practice mode created by slime (https://github.com/osm/ktx/tree/tot).
ToT is a dmm4 1v8 bot game. You spawn with full health and unlimited ammo while the bots spawn with no armor and only shotguns. The goal is to frag as many bots as possible before dying, up to a 5 minute limit. Powerups are enabled.
To play (e1m2 is a good map for this):
/tot
/botcmd enable
/botcmd fill 20
/ready