Skip to content

Conversation

@CrimRecya
Copy link
Contributor

@CrimRecya CrimRecya commented Dec 15, 2024

[ ] 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.
  • 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 range.
  • AllowDistributionCommand.SpreadMode & AllowDistributionCommand.FilterMode allow you to set spread range and target filter by hotkeys, which default to DefaultDistributionSpreadMode and DefaultDistributionFilterMode.
    • When the range is 0, it is the original default behavior of the game. The range can be adjusted to 4, 8 or 16 cells by another shortcut key. You can also adjust this by using the mouse wheel while holding down the specific hotkey if AllowDistributionCommand.SpreadModeScroll set to true;
      • The targets within the 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.
    • When the filter is 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 identical Armor) 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 same GroupAs) will be selected among the targets allocated in the range. At this time, a red ring will be displayed.
  • AllowDistributionCommand.AffectsAllies & AllowDistributionCommand.AffectsEnemies allow the distribution command to work on allies (including owner) or enemies target. If picking a target that's not eligible, it'll fallback to vanilla command.
  • It's possible to add a button for distribution mode in the bottom bar by adding DistributionMode in the ButtonList of AdvancedCommandBar and MultiplayerAdvancedCommandBar.
    • The positions of each button are hardcoded, so it'll only decide whether enable this button or not. Distribute Mode button is now always listed after all the vanilla ones.
    • The asset of these buttons should be added in sidec0x.mix files which correspond to different sides, with the name button12.shp.
  • For localization add 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:DistributionMode into your .csf file.

In rulesmd.ini:

[GlobalControls]
AllowSwitchNoMoveCommand=false                      ; boolean
AllowDistributionCommand=false                      ; boolean
AllowDistributionCommand.SpreadMode=true            ; boolean
AllowDistributionCommand.SpreadModeScroll=true      ; boolean
AllowDistributionCommand.FilterMode=true            ; boolean
AllowDistributionCommand.AffectsAllies=true         ; boolean
AllowDistributionCommand.AffectsEnemies=true        ; boolean

[AudioVisual]
StartDistributionModeSound=                         ; sound entry
EndDistributionModeSound=                           ; sound entry
AddDistributionModeCommandSound=                    ; sound entry

In ra2md.ini:

[Phobos]
DefaultApplyNoMoveCommand=true                      ; boolean
DefaultDistributionSpreadMode=2                     ; integer, 0 - r=0 , 1 - r=4 , 2 - r=8 , 3 - r=16
DefaultDistributionFilterMode=2                     ; integer, 0 - None , 1 - Like , 2 - Type , 3 - Name

In uimd.ini:

[AdvancedCommandBar]
ButtonList=[Button1],DistributionMode,[ButtonX]     ; List of button entry

[MultiplayerAdvancedCommandBar]
ButtonList=[Button1],DistributionMode,[ButtonX]     ; List of button entry
2024-12-16.01-47-48.mp4

@github-actions
Copy link

github-actions bot commented Dec 15, 2024

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.

Copy link
Contributor

@TaranDahl TaranDahl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@CrimRecya
Copy link
Contributor Author

CrimRecya commented Dec 16, 2024

In theory, it should work normally in multi-players, but I haven't tested it yet. It would be great if it could work as expected.
In addition, as for the display of the current mode, the scheme has not been finalized yet. At present, I want it to be displayed directly at the mouse position, such as the display range of SW, instead of using print message like in video. If anyone has any good ideas, PLEASE comment.

@CrimRecya CrimRecya changed the title Distribution click action mode [Highly Customized] Distribution click action mode Dec 26, 2024
@TaranDahl
Copy link
Contributor

Tested locally. Doing good.

@Metadorius
Copy link
Member

Metadorius commented Feb 10, 2025

I am hesitant on how to deal with this feature. It removes a huge amount of microcontrol from the game, essentially automating half of whatever required skill before and now just requires a mouse click.

@TaranDahl
Copy link
Contributor

I am hesitant on how to deal with this feature. It removes a huge amount of microcontrol from the game, essentially automating half of whatever required skill before and now just requires a mouse click.-----------谷歌翻译-----------我对如何处理此功能感到犹豫。它从游戏中删除了大量的微控制,从本质上讲是在此之前所需的任何技能的一半,现在只需单击鼠标即可。

I think all you want is a toggle?

@Metadorius
Copy link
Member

I would say that if we will merge it, a toggle is a must, because the whole existence of such a hotkey alters gameplay significantly, removing a huge amount of microcontrol requireemtn.

The bigger question is where do we draw the line on what should be included in mainline Phobos and what not? In other words, what is the scope of the project?

This is an exaggeration, but just to show the point that I am trying to make -- imagine suddenly a pull request appears that is intended specifically to turn YR into a whole different genre of game (say, a visual novel with no RTS stuff at all) and is not usable for purposes of regular RTS players or modders. Should we merge it at all? I don't think we should, it is, as we say, out of scope.

The main question is where do we draw the line between such features that are out of scope and the features that should be merged and are in the scope? And what potential drawbacks are there if we merge it?

@TaranDahl
Copy link
Contributor

The main question is where do we draw the line between such features that are out of scope and the features that should be merged and are in the scope?

Personally, I came to Phobos because everyone knows Phobos. If I get something done here, then people will know it's done and other people don't need to spend time on what I've already done. I think it is meaningful.
So for me, as long as I think this feature is likely to be used by others, then it is not out of scope.

And what potential drawbacks are there if we merge it?

Negligible performance impact.
Careless devs can introduce issues that can cause crashes even if features are not enabled.

@TaranDahl
Copy link
Contributor

If you have to decide on a scope, then we can vote on those PRs that anyone says are out of scope.

@Metadorius
Copy link
Member

as for the display of the current mode, the scheme has not been finalized yet. At present, I want it to be displayed directly at the mouse position, such as the display range of SW, instead of using print message like in video. If anyone has any good ideas, PLEASE comment.

I have an idea.

First, the radius can be controlled not via hotkeys, but with a mouse. How that would work? You press a hotkey to activate distribution mode, and when you press right mouse button on the target and drag your mouse without releasing the mouse button - it displays the circle and changes the radius depending on how far you drag the cursor, so you regulate the radius with the mouse drag. Then when you release the mouse - the orders are given to every selected unit. You do that each time and effectively draw circles to give orders.

This is inspired by Call to Arms: Gates of Hell game. There you can spread your infantry among covers this way, click behind covers and drag to spread people better.

@CrimRecya
Copy link
Contributor Author

I would say that if we will merge it, a toggle is a must, because the whole existence of such a hotkey alters gameplay significantly, removing a huge amount of microcontrol requireemtn.

Yes, I think we still need a toggle to enable this feature, not just defining buttons. I will add later.

The bigger question is where do we draw the line on what should be included in mainline Phobos and what not? In other words, what is the scope of the project?

Personally, if someone (whether they are modders or task package authors) needs a certain feature and I happen to be interested in it, I will try to do it. Whether to merge or not is actually related to whether more people will use it. But no matter what, these features will only be pushed if there is a demand.

This is an exaggeration, but just to show the point that I am trying to make -- imagine suddenly a pull request appears that is intended specifically to turn YR into a whole different genre of game (say, a visual novel with no RTS stuff at all) and is not usable for purposes of regular RTS players or modders. Should we merge it at all? I don't think we should, it is, as we say, out of scope.

If it is assessed that this should not be merged, then I will close it.

The main question is where do we draw the line between such features that are out of scope and the features that should be merged and are in the scope? And what potential drawbacks are there if we merge it?

As TaranDahl said, for example, if this feature does not have any impact on performance and can be disabled (assuming I have already added the switch), then I think it can be merged.

@CrimRecya
Copy link
Contributor Author

The opening video is based on the initial design, which is now outdated. I will share another one when I have time.

Copy link
Contributor

@Coronia Coronia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as per discussed with CrimRecya, it already has most of the feature done, and the requested things can be added without needing to alter the existing stuffs. Better merge this first and do further tweak if needed

@Coronia Coronia added the Tested label Oct 29, 2025
@Coronia Coronia merged commit caf18a0 into Phobos-developers:develop Oct 29, 2025
9 checks passed
@Metadorius
Copy link
Member

Metadorius commented Oct 29, 2025

Why was this merged? This is a T2 request, and no T2 maintainer gave approval or even reviewed the code. My feedback wasn't addressed in full and I haven't expressed any agreement with dismissing it.

@DeathFishAtEase
Copy link
Collaborator

Since this PR is labeled as T2 maintainer review is sufficient, it is indeed better for another T2 maintainer to review the code first and confirm approval.

@Coronia
Copy link
Contributor

Coronia commented Oct 30, 2025

I think I've already written part of my reasons above, but to be clarified I'd like to list it one more time:

  • main reason is that I don't think all things need to be done in one PR, but can be gradually implemented pieces by pieces. Otherwise, it'll either take more time to implement, review and test, but also have more risk to become the 'big bad PR' we don't want
  • follow that, I've checked the current codes and review records and confirmed that there's no word related to the implementation itself. What @Metadorius suggested was all about adding more features, which @CrimRecya has confirmed that it can be compatible with current design
  • Crim also stated the same idea: this batch could be merged first and had more features added in the future (and he has also consulted you for that). And given he might not have time for it, others can help with these features once it's been merged, like I'll try to finish some of the things that's been mentioned above in Distribution Mode Extension #1935
  • and on top of that, this PR has gone through many test already in both single and multiplayer, so its correctness and stability can be ensured. And since it's a standalone feature that needs enabling and not interfering others, it won't pose any issue to other things ingame either

To sum up, I think it would be better to not let PR stay in the fridge for too long (this one has been almost a year). As long as its tag design is extendible, being test sufficiently and not affecting existing things if not enabled, it could be merged first and maintained further in the future with more than one people's efforts (which is also my opinion towards the New Trajectory PR, if you've read it)

I do apologize for merging it without a T2 review tho and wouldn't do it next time, but please take my word into consideration cuz I think it's the better way for Phobos

@ZivDero
Copy link
Contributor

ZivDero commented Oct 30, 2025

@Coronia CC @CrimRecya

You seem to misunderstand how collaborative projects work. If a senior team member (let alone, project lead) tells you that a change has to happen, is necessary, or a PR is otherwise unfitting, then it has to happen. What you do not do is go around their back and force whatever you think is right into the project. If you think that your counterpart is mistaken, you can try to convince them, but ultimately it is their decision as to what happens and what doesn't happen.

I understand your concerns regarding potentially long feature development times. However, it is the feature author's burden to develop the feature, request reviews and address any concerns in a timely manner. While there are not many T2 maintainers, I don't believe you have exhausted all opportunities to get a review.

That said, what happened is absolutely unacceptable and you've clearly overstepped your boundaries. Everyone pushes features they are interested in, and that is normal, but forcing the issue beyond your permissions without upper management's consent is a breach of trust.

@Metadorius
Copy link
Member

Metadorius commented Oct 30, 2025

Treat the majority of what @ZivDero has written above as my words too.

@CrimRecya @Coronia

(which is also my opinion towards the New Trajectory PR, if you've read it)

Talking about new trajectories PR. Seeing the difficulties with acceptance of our feedback, both me and @ZivDero were trying a diplomatic approach in how to convey it as a friendly way of resolving the issue. I stopped responding because I felt bad about the conflict and didn't have the energy at the time to continue swallowing disregard for the sake of smooth resolution of the issue.

We thought that the authority of myself, @Starkku and @ZivDero, as well as other maintainers together, is at least somewhat clear, and that it does not need to be brought to the conversation, but first your responses with borderline disrespectful comments about "losing patience", "wasting time" and now what happened here yesterday (and especially that) suggests that no, I should have been direct and explicit without diplomatic niceties, so let me state it clearly now.

You do not have the right to dismiss the unresolved critical feedback of other maintainers of equal or higher status, or endlessly argue about it without resolution, wasting their time and patience. Yes, this is a community project, but we still have an organizational structure, and being a community project doesn't grant you automatic rights to dismiss feedback, especially feedback about core architectural issues, like Starkku's feedback about trajectories (especially considering he's the most experienced Phobos maintainer), or decide on your own what's best for Phobos and for specific PRs.

Metadorius added a commit that referenced this pull request Oct 30, 2025
@TaranDahl
Copy link
Contributor

我想说,问题的根源在于Phobos的pr审核效率仍然低下得令人发指。他们说 "losing patience" "wasting time" 不是不尊重你,他们只是在陈述事实,因为事实就是你们在轨迹PR上花费了太多的时间。
pr审核效率低下得根本原因在于我们的项目缺少明确的、能在确定的时间内结束的pr审核流程。
当一个新的pr出现的时候,谁来审核?谁来测试?不知道,谁爱来谁来。这个pr可能我感兴趣那我来,下个pr可能Recya感兴趣那Recya来,但如果没人感兴趣怎么办?那你就等吧,一等一个不吱声,冰箱里最早的pr是2021年,如果算上之前因为没有继续维护而被关闭的那还要更早。
既然Phobos是一个社区项目,接受社区提供的pr,那就得有一个确定性的pr处置流程,不能全靠维护者自发地审核。
I want to say that the root of the problem lies in Phobos' PR review efficiency, which is still shockingly low. They say 'losing patience' and 'washing time' is not disrespectful to you, they are just stating the fact that you have spent too much time on trajectory PR.
The fundamental reason for the low efficiency of PR review is that our project lacks a clear PR review process that can be completed within a certain time frame.
Who will review a new PR when it appears? Who will test it? No one knows, who loves who comes. If I am interested in this PR, then I will do it. If Recya is interested in the next PR, then Recya will do it. But what if no one is interested? Then you can go and wait, with no ending time. The earliest PR in the refrigerator was in 2021, and if we include those that were closed before due to lack of continued maintenance, it will be even earlier.
Since Phobos is a community project that accepts PR provided by the community, there must be a deterministic PR disposal process that cannot rely solely on maintainers' spontaneous review.

@Metadorius
Copy link
Member

Metadorius commented Oct 30, 2025

I want to say that the root of the problem lies in Phobos' PR review efficiency

While I agree it is a problem, this is not what is happening here, nor it was what was happening in the #1582. The review feedback was given, but instead of being addressed it was dismissed multiple times.

They say 'losing patience' and 'washing time' is not disrespectful to you, they are just stating the fact that you have spent too much time on trajectory PR.

That is not stating the fact. Stating the fact would be "We have spent a lot of time on this PR". What was said instead was disrespectful, when we were actively trying to be diplomatic, the other side didn't care on the same level about a choice of words.

The fundamental reason for the low efficiency of PR review is that our project lacks a clear PR review process that can be completed within a certain time frame.

No, the fundamental reason is the lack of active T2 maintainers and slow "growth" of new T2 maintainers, which is the cause of slow review process. Yes, it's sad that it's slow, but there's no magic to just get more maintainers or magically transfer more skills and experience to others to be able to handle the project on a higher level, and I am not willing to shoot the project in the foot by accepting code with major architectural issues. We neither lose any money, nor endanger someone's lives, nor have any other life dependency, nor do we even have deadlines to warrant that, but we do have reputation and standards to uphold, which is my responsibility to do, ultimately.

@Metadorius
Copy link
Member

Metadorius commented Oct 30, 2025

And for the record, we always had issues with reviewing and housekeeping. My contribution list is so small not because I am doing nothing, but because I spent more time than anyone else reviewing things in the past, to the point of burning out multiple times, because no one ever wanted to actively review and housekeep things, but everyone wanted to write them. Credits where it's due, you guys have been more actively doing that, which is the reason we went from 140 PRs to 70 PRs, which was a huge relief, but alas, now Starkku and ZivDero are busy elsewhere, I am working on completing things related to Ares/Phobos/new spawner update for CnCNet, which means we lack people on T2+ level.

It is a problem that ideally requires a solution, but what has been done is not an acceptable solution to that.

@TaranDahl
Copy link
Contributor

No, the fundamental reason is the lack of active T2 maintainers and slow "growth" of new T2 maintainers, which is the cause of slow review process.

我去看了Teams,我们有8位T2维护者。即使现在有4位因为各种原因而无法审查PR,还有另外4位。他们在做什么?
I checked Teams. We have 8 T2 maintainers. Even if 4 of them are unable to review PRs for various reasons at the moment, there are still 4 others. What are they doing?

@Metadorius
Copy link
Member

我去看了Teams,我们有8位T2维护者。即使现在有4位因为各种原因而无法审查PR,还有另外4位。他们在做什么? I checked Teams. We have 8 T2 maintainers. Even if 4 of them are unable to review PRs for various reasons at the moment, there are still 4 others. What are they doing?

The teams are not representative of active maintainers. For the past years I've been executing the policy of keeping historical roles of people and not removing them from the team out of courtesy for their past work. It's a hobby after all, maybe they'll want to return some day and it would be mentally easier for them to do so.

See active maintainers in the README.md instead.

@TaranDahl
Copy link
Contributor

我们当然应该记住那些曾经做过贡献的人,但我认为你将他们放在T2这个标签里是不合适的,毕竟我们需要这个标签来审核PR。你可以创建一个新的,比如"Honorable Retired"之类的。
Of course, we should remember those who have made contributions. However, I think it's inappropriate for you to put them under the T2 label. After all, we need this label to review PRs. You can create a new one, such as "Honorable Retired" or something like that.

@Metadorius
Copy link
Member

Maybe, however, let's not stray away from the core issue.

@TaranDahl
Copy link
Contributor

TaranDahl commented Oct 30, 2025

Yeah. I will discuss that with you privately later.

Forget it. There's nothing more to discuss.
I was going to draft a plan for the review cycle. Then I realized that we only have 7 people in total reviewing the code😅
Given this number, there's nothing to blame for the current speed.

@ZivDero
Copy link
Contributor

ZivDero commented Oct 30, 2025

@TaranDahl

there are still 4 others. What are they doing?

I dunno? Did you ask them? Yes, in an ideal world reviewers come by themselves. But as you've said, we're very short on manpower. I have TS, Starkku has PP. However, if the authors asked either of us, I'm sure they would've received feedback. Maybe not immediately. Maybe it'd take a few weeks even. But within the lifespan of this PR, even that seems pretty short.

So don't hesitate to ask directly. We're a tightly knit community, and the situation dictates that it's appropriate and even necessary.

Metadorius added a commit that referenced this pull request Oct 30, 2025
…ion mode (#1453)"

This reverts commit caf18a0.
The PR was merged prematurely.
@Metadorius
Copy link
Member

This PR has to be reopened to facilitate proper process, as we, of course, don't want to stop it from being merged eventually.

@CrimRecya CrimRecya deleted the develop-Distribution branch November 19, 2025 15:07
Coronia pushed a commit to Coronia/Phobos that referenced this pull request Nov 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

❓New feature ⚙️T2 T2 maintainer review is sufficient Tested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants