Distribution click action mode - #1993
Hidden character warning
Conversation
|
Nightly build for this pull request:
This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build. |
|
@Metadorius Any issue other than #1949 (review)? |
|
TODO:
@Metadorius Please confirm the above summary, or supplement/correct the incorrect parts. |
|
@TaranDahl yeah, correct, there was also a comment about implementing press-and-drag mode (and not sure if the "same type" (infantry/buildings/vehicles/etc) is needed, since we have "same armor" mode). CrimRecya said it's too problematic, however I don't really see why, since we already have drag selection and we could reuse drag selection to calculate the radius. |
I think there is not enough labor force to add more features. |
This isn't a feature though? It is just a somewhat small improvement that brings it in line with how modern games do it.
From that you could calculate 2 points, get world coords via function above, use this info to draw a corresponding circle and set the mode to such. I am not sure what is complex here. If needed I can send my decompile for this. |
|
@Metadorius Is there any difference between |
|
@TaranDahl |
Judging from the current code, there seems to be no need for it to inherit from |
Which is a bad pattern. You have class fields and methods for that. |
|
Yeah I will make a new class to arrange the new buttons. But for the vanilla buttons, maybe we should just let them be? |
Yeah I didn't mean we should be squeezing vanilla static array shitcode into proper classes necessarily, should be good. |
|
If we want to implement dragging, how should we handle the selection range? |
update update doc 禁止分配到回收站 update update Update src/Commands/DistributionMode.cpp fix core disable force attack for distribution mode fix double keys breaking distribution command fix distribution mode crash add AffectsNeutral for distribution mode Update Phobos.vcxproj move things to new files update doc Button on bottom TODO Fit with FakeOf Fix include Separate functions Fix default value Remove useless Refactor remove unnecessary Grinder process Code style ... Low distance first Fix merge Message, sound and scroll action Global toggle Fix target outside units Fix target disguised units Fix target shrouded units Doc Add a hold down key to enable Fix target cloaked units Fix a typo Show range ring Draw at mouse Core Co-Authored-By: Kerbiter <crabiter@vivaldi.net> Co-Authored-By: Coronia <28055583+Coronia@users.noreply.github.com> Co-Authored-By: 绯红热茶 <169989423+CrimRecya@users.noreply.github.com>
|
maybe the default also the upper limit of as for the drag, I think instead of a choice between it and scroll, it'd better just make a toggle in rulesmd.ini to enable or disable it, like |
Changed. |
Metadorius
left a comment
There was a problem hiding this comment.
I did a doc pass, sorry, haven't got to the rest yet. treat it both as a doc review (if what I asked for is true but docs are not reflecting it yet) and design review (if what I asked about is not true yet)
|
|
||
| ### `[ ]` Distribution Mode Spread / Filter / Enable | ||
|
|
||
| - Now you can change the click action by using `AllowSwitchNoMoveCommand` hotkey. If the behavior to be executed by the current techno is different from the behavior displayed by the mouse, and the behavior to be executed will make the techno move near the target, the behavior will be replaced with area guard. Regardless of whether or not switch hotkey is used, default behavior can be changed through `DefaultApplyNoMoveCommand`. |
There was a problem hiding this comment.
- I don't think this has to be in Distribution Mode docs
- it's not clear what this does, is it a toggle for "fallback" behavior if different from the mouse one and allows disabling it? if so - maybe
UnavailableActionFallsBackToMove=actionwill be more intuitive? orUnavailableActionFallBack=none/action(defaults to move), with customisability per type (not sure what handles actions, MissionType?) maybe useful?
There was a problem hiding this comment.
- I don't think this has to be in Distribution Mode docs
wdym? A function without docs?
2. it's not clear what this does, is it a toggle for "fallback" behavior if different from the mouse one and allows disabling it? if so - maybe will be more intuitive? or (defaults to move), with customisability per type (not sure what handles actions, MissionType?) maybe useful?
UnavailableActionFallsBackToMove=action``UnavailableActionFallBack=none/action
The current fallback behavior is not simply changing the Action, but a specially customized behavior targeting the AreaGuard unit itself instead of the target.
There was a problem hiding this comment.
wdym? A function without docs?
no, just a separate section. it's not really relevant to distribution mode and can be used without it, right? it's just a complementary function
There was a problem hiding this comment.
The current fallback behavior is not simply changing the Action, but a specially customized behavior targeting the AreaGuard unit itself instead of the target.
then the tag name is still not clear IMO
|
|
||
| - Now you can change the click action by using `AllowSwitchNoMoveCommand` hotkey. If the behavior to be executed by the current techno is different from the behavior displayed by the mouse, and the behavior to be executed will make the techno move near the target, the behavior will be replaced with area guard. Regardless of whether or not switch hotkey is used, default behavior can be changed through `DefaultApplyNoMoveCommand`. | ||
| - Now you can also change the click action when hold down the specific hotkey if enabled `AllowDistributionCommand`. The new behavior is like using the selected objects one by one to click on each target within the spread range. | ||
| - The targets within the spread range will be allocated equally to the selected technos. Only when the behavior to be performed by the current techno is the same as that displayed by the mouse will it be allocated. Otherwise, it will return to the original default behavior of the game (it will not be effective for technos in the air). This will display a range ring. |
There was a problem hiding this comment.
Otherwise, it will return to the original default behavior of the game
do you mean the move fallback? if so - you should mention that and/or link it
(it will not be effective for technos in the air)
hmm?
| - Now you can change the click action by using `AllowSwitchNoMoveCommand` hotkey. If the behavior to be executed by the current techno is different from the behavior displayed by the mouse, and the behavior to be executed will make the techno move near the target, the behavior will be replaced with area guard. Regardless of whether or not switch hotkey is used, default behavior can be changed through `DefaultApplyNoMoveCommand`. | ||
| - Now you can also change the click action when hold down the specific hotkey if enabled `AllowDistributionCommand`. The new behavior is like using the selected objects one by one to click on each target within the spread range. | ||
| - The targets within the spread range will be allocated equally to the selected technos. Only when the behavior to be performed by the current techno is the same as that displayed by the mouse will it be allocated. Otherwise, it will return to the original default behavior of the game (it will not be effective for technos in the air). This will display a range ring. | ||
| - `DefaultDistributionSpreadRange` controls the initial spread range, which is a number that's corresponding to the amount of cell radius * 512. |
There was a problem hiding this comment.
we should probably move the modes description earlier
| - `DefaultDistributionSpreadRange` controls the initial spread range, which is a number that's corresponding to the amount of cell radius * 512. | |
| - `DefaultDistributionSpreadRange` controls the initial spread range for the scrollwheel mode, which is a number that's corresponding to the amount of cell radius * 512. |
There was a problem hiding this comment.
It applies to all modes except PressAndDrag. I don't think it needs special explanation, because it should be intuitive that PressAndDrag does not take this into account.
| AllowDistributionCommand.SpreadModeHotKey=true ; boolean | ||
| AllowDistributionCommand.SpreadModeScroll=true ; boolean | ||
| AllowDistributionCommand.SpreadModeDrag=true ; boolean |
There was a problem hiding this comment.
I don't think it should be controlled from rules.ini. Control schemes are not up to mod author.
The mod author should be able to enable or disable the feature (because it significantly alters the gameplay), but the players must be able to choose their preferred implementation themselves via RA2MD.ini.
| StartDistributionModeSound= ; sound entry | ||
| EndDistributionModeSound= ; sound entry |
There was a problem hiding this comment.
let's default this to the same sound as waypoint mode
| [AudioVisual] | ||
| StartDistributionModeSound= ; sound entry | ||
| EndDistributionModeSound= ; sound entry | ||
| AddDistributionModeCommandSound= ; sound entry |
| - `AllowDistributionCommand.UseClick` controls whether distribution mode is activated by clicking on a target. When set to false, distribution mode only works via press-and-drag, and the range ring will only be shown while dragging. | ||
| - You can also adjust spread range by using the mouse wheel while holding down the specific hotkey if `AllowDistributionCommand.SpreadModeScroll` set to true. This allows a more precise control of spread range that each step will increase/decrease it by `DistributionSpreadScrollStep`, with 20 cells as its maximum value. | ||
| - `AllowDistributionCommand.SpreadModeDrag` allows you to adjust the spread range by pressing and dragging the mouse while holding down the specific hotkey. The drag distance from the starting point determines the spread range. This can naturally co-exist with `SpreadModeScroll`. | ||
| - `AllowDistributionCommand.FilterMode` allows you to set target filter by hotkey, which default to `DefaultDistributionFilterMode`. |
There was a problem hiding this comment.
I am not sure how this tag works? at least from the documentation
There was a problem hiding this comment.
Set it to false, and it will function exactly as you described earlier. It is fully tailored for you.
There was a problem hiding this comment.
so this is a toggle between togglable and not togglable mode? I am not sure why this needs to be customised at rulesmd.ini level though?
There was a problem hiding this comment.
Are you asking about the UseClick or FilterMode?
| - Deselect 1 or 5 object(s) from current selected objects. | ||
| - For localization add `TXT_DESELECT`, `TXT_DESELECT_DESC`, `TXT_DESELECT5` and `TXT_DESELECT5_DESC` into your `.csf` file. | ||
|
|
||
| ### `[ ]` Distribution Mode Spread / Filter / Enable |
There was a problem hiding this comment.
I wonder if we could assign it by default to something like C? or maybe (crazy idea) holding X (and making specifically the release of the same hotkey in a short time scattering)?
There was a problem hiding this comment.
I believe the current architecture does not allow the same key to be used for different functions.
There was a problem hiding this comment.
waypoint mode somehow works like this, so this should too
There was a problem hiding this comment.
waypoint mode somehow works like this, so this should toowaypoint mode somehow works like this, so this should too
I don't quite know what you're referring to.
By default, the hotkey for waypoint mode is Z, and the Z key is exclusively used by waypoint mode.
It is not feasible under the current hotkey architecture for different CommandClass to share the same hotkey and distinguish between short presses and long presses only. Also I don't think it is worthwhile to significantly expand the existing hotkey architecture for this at the moment; it would make more sense to devote that effort to the development of OpenYR.
There was a problem hiding this comment.
oh I confused the response thread, nevermind. just ensure it works like waypoint mode hotkey, that's all
Co-authored-by: Kerbiter <crabiter@vivaldi.net>
Co-authored-by: Kerbiter <crabiter@vivaldi.net>
# Conflicts: # CREDITS.md # docs/User-Interface.md # src/Ext/TechnoType/Body.h # src/Phobos.INI.cpp # src/Phobos.h
|
@Metadorius How about the rest of your review? |
Have you addressed the design changes I requested? without that it would be a moot point to get deeper into code |
So there are no further issues with the design, right? |
you left some of my comments/suggestion unaddressed though, either by changes or replies, that's what I was pointing at |
Yeah I know it. I was looking forward to some other reviews, to deal with them together. |
# Conflicts: # src/Ext/TechnoType/Body.cpp

Reopen #1453 again.
[ ]Distribution Mode Spread / Filter / EnableAllowSwitchNoMoveCommandhotkey. If the behavior to be executed by the current techno is different from the behavior displayed by the mouse, and the behavior to be executed will make the techno move near the target, the behavior will be replaced with area guard. Regardless of whether or not switch hotkey is used, default behavior can be changed throughDefaultApplyNoMoveCommand.AllowDistributionCommand. The new behavior is like using the selected objects one by one to click on each target within the spread range.DefaultDistributionSpreadRangecontrols the initial spread range, which is a number that's corresponding to the amount of cell radius * 512.AllowDistributionCommand.SpreadModeallows you to set spread range by hotkey. There're 4 tiers of range that can be selected by this hotkey which are identical to 0, 4, 8 and 16 cells.AllowDistributionCommand.SpreadModeScrollset to true. This allows a more precise control of spread range that each step will increase/decrease it byDistributionSpreadScrollStep, with 20 cells as its maximum value.AllowDistributionCommand.FilterModeallows you to set target filter by hotkey, which default toDefaultDistributionFilterMode.None, it is the default behavior of the game. If the range is not zero at this time, a green ring will be displayed. You can adjust the filter mode to:Like- only targets with the same armor type (Completely identicalArmor) will be selected among the targets allocated in the range. At this time, a blue ring will be displayed.Type- only targets of the same type (like infantries, vehicles or buildings) will be selected among the targets allocated in the range. At this time, a yellow ring will be displayed.Name- only targets of the same name (or with the sameGroupAs) will be selected among the targets allocated in the range. At this time, a red ring will be displayed.AllowDistributionCommand.AffectsAllies,AllowDistributionCommand.AffectsEnemies&AllowDistributionCommand.AffectsNeutralallow the distribution command to work on allies (including owner), enemies or neutral target. If picking a target that's not eligible, it'll fallback to vanilla command.DistributionModein theButtonListofAdvancedCommandBarandMultiplayerAdvancedCommandBar.sidec0x.mixfiles which correspond to different sides, with the namebutton12.shp.TXT_SWITCH_NOMOVE,TXT_DISTR_SPREAD,TXT_DISTR_FILTER,TXT_DISTR_HOLDDOWN,TXT_SWITCH_NOMOVE_DESC,TXT_DISTR_SPREAD_DESC,TXT_DISTR_FILTER_DESC,TXT_DISTR_HOLDDOWN_DESC,MSG:DistributionModeOn,MSG:DistributionModeOff,TIP:DistributionModeinto your.csffile.In
rulesmd.ini:In
ra2md.ini:In
uimd.ini: