Update with more commands, some more functionality, some fixes #195
Conversation
|
There's a lot here, so it will take time to fully review. One thing I'll say for now is that I fixed the |
|
Yeah there's quite a lot. Big theme is just ease-of-use hope that's appreciated. I use stuff like I'll put "spawn_as" the default to permanent if this was seen as more of a bug fix anyways. Don't know if spawning as something temporarily would see much use but probably fine to have it as a an option. |
It is indeed appreciated. I'm still going through the review and so far there only a few objections on either the implementation of something or whether something should be included. I expect the other members to offer their opinion if there is a disagreement, so don't take everything I say at face value. But brace yourself for A LOT of comments, mainly for code quality and some minor issues. I might leave some classes for a second round of reviewing just so I can get the rest to you in a prompt manner.
Yep, it was just one-line bug fix. Just because an extra option is now available, it doesn't always mean it adds any real value if it's too niche. Scope creep vs simplicity/functionality and all. |
SChinchi
left a comment
SChinchi
left a comment
There was a problem hiding this comment.
I'm submitting this review as general feedback, but there are some issues that should be addressed before merging. For the record the following classes have not been thoroughly reviewed as I lack the time and the mental energy at the moment. Expect a follow-up in a few days:
- Buffs
- Items
- PlayerCommands
- Hooks
- Lang
- StringFinder
|
Removed duplicate 'list_drones' & 'give_drone' Returned GIVEOBJECT, 'droptable' 'Kill_all' default now just calls 'kill_all 2' & '4' instead of doubled code. 'no_interactables' now works with simu 'dtpeace' now does 'god 1' Now getting modded CSCs for list_directorcards the same way. 'evolve_lemurians [target (player|'all'):]' & making command in line with rest of mod. 'kill_all_minions' now says it works with pinged fixed 'rich/poor' getting player from wrong arg Renamed 'list_mods' to 'dump_mods', proper help, removed args. 'give_void' no longer subtracts below 0 Removed 'scanner' unneed alliteration Reverted 'spawn_as' addition of 'permanent' argument for being too, unneeded. Added 'godenemy' for parrity with 'buddhaenemy', even if use case is beyond me. Fixed altered crit not being reset by 'reset_stats' 'spawn_interactable' amount now gotten same way other amounts are gotten in mod Reblocked vanilla 'timescale', wasn't meant to go through. GetDrone & GetPickup from partial documentation. 'give_equip -1' now shows in auto complete. 'goto_boss' no longer usable outside of run, no longer has wrong help. Made default 'random_items' weighted like the vanilla randomitems method |
cleaner code this way. also added same support for 'remove_buff'
calls the other commands now. Apparently 'removed_all_buffs 0' is also just broken with ss2 at least with beta. Not sure what to do about that
This should help if testing smth like how does it work on eclipse vs other difficulty. Does mean the mod needs to depend on R2API_Difficulty. -For modded difficulties -For DifficultyDef to DifficutlyIndex (not invanilla I think)
Do think this is still a good command to have. Fixed Parse.BuffTarget 'pinged' returning wrong fail message if it fails.
dtdamage: Too plain unlimited_junk: Too specific to need a toggle. kill_all_minions: Seems a bit too specific, I use kill_all 1 all the time I wouldn't switch over to this.. But remove_all_drones is good.
Changed 'create_pickup' [search] to an enum Fixed an inconsistency with 'buddhaEnemy' 'charge_zone' default value 100%, instead of requiring you to give number. (Seems unnecessary to ask for number) Added dt style safety checks for 'loadout_set_skill_variant', 'skill', 'skin'
Did != instead of == oops
Devs got a command like this, good for like crafting testing
'remove_all_drones` -> `remove_all_minions` 'give_drone' -> Replaced with `spawn_minion` Maybe more agreeable overlap? And built upon already better code instead of more so duplicate. I just like being able to get the minion I want with 1 argument not 4 but that might not be too important. But other proposed commands are of similiar stature. Replaces elite with droneTier and axes the TeamIndex argument. If any other minion specific stuff might get added it could also just get added to this command yknow.
`toggle_hud` -> `hide_hud` consistency with hide_model and easier to type for me. `true_kill all` now actually does that. Lang for previous 2 commits.
2 participants
Hope more people can get to use this stuff.
If some stuff needs to be changed let me know.
Only known issue;
no_cooldownsdoesn't work with SS2 due to some old code on their end, not really anything that can be done here.