-
-
Notifications
You must be signed in to change notification settings - Fork 124
Distribution click action mode #1993
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
The head ref may contain hidden characters: "\u5206\u6563\u70B9\u51FB\u6A21\u5F0F"
Conversation
…tion # Conflicts: # CREDITS.md # docs/Whats-New.md # src/Commands/Commands.cpp # src/Phobos.INI.cpp # src/Phobos.h
…tion # Conflicts: # CREDITS.md # docs/Whats-New.md
|
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? |
| if (!DistributionModeHoldDownCommandClass::OffMessageShowed) | ||
| { | ||
| DistributionModeHoldDownCommandClass::OffMessageShowed = true; | ||
| MessageListClass::Instance.PrintMessage(GeneralUtils::LoadStringUnlessMissing("MSG:DistributionModeOff", L"Distribution mode unabled."), RulesClass::Instance->MessageDelay, HouseClass::CurrentPlayer->ColorSchemeIndex, true); |
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.
| MessageListClass::Instance.PrintMessage(GeneralUtils::LoadStringUnlessMissing("MSG:DistributionModeOff", L"Distribution mode unabled."), RulesClass::Instance->MessageDelay, HouseClass::CurrentPlayer->ColorSchemeIndex, true); | |
| MessageListClass::Instance.PrintMessage(GeneralUtils::LoadStringUnlessMissing("MSG:DistributionModeOff", L"Distribution mode disabled."), RulesClass::Instance->MessageDelay, HouseClass::CurrentPlayer->ColorSchemeIndex, true); |
Metadorius
left a comment
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.
partial review
| void __fastcall DistributionModeHoldDownCommandClass::ClickedWaypoint(ObjectClass* pSelect, int idxPath, signed char idxWP) | ||
| { | ||
| pSelect->AssignPlanningPath(idxPath, idxWP); | ||
|
|
||
| if (const auto pFoot = abstract_cast<FootClass*, true>(pSelect)) | ||
| pFoot->unknown_bool_430 = false; | ||
| } | ||
|
|
||
| void __fastcall DistributionModeHoldDownCommandClass::ClickedTargetAction(ObjectClass* pSelect, Action action, ObjectClass* pTarget) | ||
| { | ||
| pSelect->ObjectClickedAction(action, pTarget, false); | ||
| Unsorted::MoveFeedback = false; | ||
| } | ||
|
|
||
| void __fastcall DistributionModeHoldDownCommandClass::ClickedCellAction(ObjectClass* pSelect, Action action, CellStruct* pCell, CellStruct* pSecondCell) | ||
| { | ||
| pSelect->CellClickedAction(action, pCell, pSecondCell, false); | ||
| Unsorted::MoveFeedback = false; | ||
| } | ||
|
|
||
| void __fastcall DistributionModeHoldDownCommandClass::AreaGuardAction(TechnoClass* pTechno) | ||
| { | ||
| pTechno->ClickedMission(Mission::Area_Guard, reinterpret_cast<ObjectClass*>(pTechno->GetCellAgain()), nullptr, nullptr); | ||
| Unsorted::MoveFeedback = false; | ||
| } | ||
|
|
||
| DEFINE_HOOK(0x4AE7B3, DisplayClass_ActiveClickWith_Iterate, 0x0) | ||
| { | ||
| enum { SkipGameCode = 0x4AE99B }; | ||
|
|
||
| const int count = ObjectClass::CurrentObjects.Count; | ||
|
|
||
| if (count > 0) | ||
| { | ||
| { | ||
| GET_STACK(int, idxPath, STACK_OFFSET(0x18, -0x8)); | ||
| GET_STACK(unsigned char, idxWP, STACK_OFFSET(0x18, -0xC)); | ||
|
|
||
| for (const auto& pSelect : ObjectClass::CurrentObjects) | ||
| { | ||
| DistributionModeHoldDownCommandClass::ClickedWaypoint(pSelect, idxPath, idxWP); | ||
| } | ||
| } | ||
|
|
||
| GET_STACK(ObjectClass* const, pTarget, STACK_OFFSET(0x18, 0x4)); | ||
| GET_STACK(Action const, action, STACK_OFFSET(0x18, 0xC)); | ||
|
|
||
| if (pTarget) | ||
| { | ||
| const int spreadMode = Phobos::Config::DistributionSpreadMode; | ||
| const int filterMode = Phobos::Config::DistributionFilterMode; | ||
| const bool noMove = !Phobos::Config::ApplyNoMoveCommand; | ||
| const auto pTechno = abstract_cast<TechnoClass*, true>(pTarget); | ||
|
|
||
| // Distribution mode main | ||
| if (DistributionModeHoldDownCommandClass::Enabled | ||
| && spreadMode | ||
| && count > 1 | ||
| && action != Action::NoMove | ||
| && !PlanningNodeClass::PlanningModeActive | ||
| && pTechno | ||
| && !pTechno->IsInAir() | ||
| && (HouseClass::CurrentPlayer->IsAlliedWith(pTechno->Owner) | ||
| ? Phobos::Config::AllowDistributionCommand_AffectsAllies | ||
| : Phobos::Config::AllowDistributionCommand_AffectsEnemies)) | ||
| { | ||
| VocClass::PlayGlobal(RulesExt::Global()->AddDistributionModeCommandSound, 0x2000, 1.0); | ||
| const bool targetIsNeutral = pTechno->Owner->IsNeutral(); | ||
| const auto pType = pTechno->GetTechnoType(); | ||
| const int range = (2 << spreadMode); | ||
| const auto center = pTechno->GetCoords(); | ||
| const auto pItems = Helpers::Alex::getCellSpreadItems(center, range); | ||
|
|
||
| std::vector<std::pair<TechnoClass*, int>> record; | ||
| const size_t maxSize = pItems.size(); | ||
| record.reserve(maxSize); | ||
|
|
||
| int current = 1; | ||
|
|
||
| for (const auto& pItem : pItems) | ||
| { | ||
| if (pItem->IsDisguisedAs(HouseClass::CurrentPlayer)) | ||
| continue; | ||
|
|
||
| if (pItem->CloakState == CloakState::Cloaked && !pItem->GetCell()->Sensors_InclHouse(HouseClass::CurrentPlayer->ArrayIndex)) | ||
| continue; | ||
|
|
||
| auto coords = pItem->GetCoords(); | ||
|
|
||
| if (!MapClass::Instance.IsWithinUsableArea(coords)) | ||
| continue; | ||
|
|
||
| coords.Z = MapClass::Instance.GetCellFloorHeight(coords); | ||
|
|
||
| if (MapClass::Instance.GetCellAt(coords)->ContainsBridge()) | ||
| coords.Z += CellClass::BridgeHeight; | ||
|
|
||
| if (!MapClass::Instance.IsLocationShrouded(coords)) | ||
| record.emplace_back(pItem, 0); | ||
| } | ||
|
|
||
| const size_t recordSize = record.size(); | ||
| std::sort(&record[0], &record[recordSize],[¢er](const auto& pairA, const auto& pairB) | ||
| { | ||
| const auto coordsA = pairA.first->GetCoords(); | ||
| const double distanceA = Point2D{coordsA.X, coordsA.Y}.DistanceFromSquared(Point2D{center.X, center.Y}); | ||
|
|
||
| const auto coordsB = pairB.first->GetCoords(); | ||
| const double distanceB = Point2D{coordsB.X, coordsB.Y}.DistanceFromSquared(Point2D{center.X, center.Y}); | ||
|
|
||
| return distanceA < distanceB; | ||
| }); | ||
|
|
||
| for (const auto& pSelect : ObjectClass::CurrentObjects) | ||
| { | ||
| size_t canTargetIndex = maxSize; | ||
| size_t newTargetIndex = maxSize; | ||
|
|
||
| for (size_t i = 0; i < recordSize; ++i) | ||
| { | ||
| const auto& [pItem, num] = record[i]; | ||
|
|
||
| if (pSelect->MouseOverObject(pItem) != action) | ||
| continue; | ||
|
|
||
| if (!targetIsNeutral && pItem->Owner->IsNeutral()) | ||
| continue; | ||
|
|
||
| if (filterMode) | ||
| { | ||
| const auto pItemType = pItem->GetTechnoType(); | ||
|
|
||
| if (!pItemType) | ||
| continue; | ||
|
|
||
| if (TechnoTypeExt::ExtMap.Find(pType)->FakeOf != pItemType | ||
| && TechnoTypeExt::ExtMap.Find(pItemType)->FakeOf != pType) | ||
| { | ||
| if (filterMode == 1) | ||
| { | ||
| if (pItemType->Armor != pType->Armor) | ||
| continue; | ||
| } | ||
| else if (filterMode == 2) | ||
| { | ||
| if (pItem->WhatAmI() != pTechno->WhatAmI()) | ||
| continue; | ||
| } | ||
| else // filterMode == 3 | ||
| { | ||
| if (TechnoTypeExt::GetSelectionGroupID(pItemType) != TechnoTypeExt::GetSelectionGroupID(pType)) | ||
| continue; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| canTargetIndex = i; | ||
|
|
||
| if (num < current) | ||
| { | ||
| newTargetIndex = i; | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| if (newTargetIndex == maxSize && canTargetIndex != maxSize) | ||
| { | ||
| ++current; | ||
| newTargetIndex = canTargetIndex; | ||
| } | ||
|
|
||
| if (newTargetIndex != maxSize) | ||
| { | ||
| auto& [pNewTarget, recordCount] = record[newTargetIndex]; | ||
|
|
||
| DistributionModeHoldDownCommandClass::ClickedTargetAction(pSelect, action, pNewTarget); | ||
|
|
||
| ++recordCount; | ||
| continue; | ||
| } | ||
|
|
||
| const auto currentAction = pSelect->MouseOverObject(pTechno); | ||
|
|
||
| if (noMove && currentAction == Action::NoMove && (pSelect->AbstractFlags & AbstractFlags::Techno) != AbstractFlags::None) | ||
| DistributionModeHoldDownCommandClass::AreaGuardAction(static_cast<TechnoClass*>(pSelect)); | ||
| else | ||
| DistributionModeHoldDownCommandClass::ClickedTargetAction(pSelect, currentAction, pTechno); | ||
| } | ||
| } | ||
| else | ||
| { | ||
| for (const auto& pSelect : ObjectClass::CurrentObjects) | ||
| { | ||
| const auto currentAction = pSelect->MouseOverObject(pTarget); | ||
|
|
||
| if (noMove && action != Action::NoMove && currentAction == Action::NoMove && (pSelect->AbstractFlags & AbstractFlags::Techno) != AbstractFlags::None) | ||
| DistributionModeHoldDownCommandClass::AreaGuardAction(static_cast<TechnoClass*>(pSelect)); | ||
| else | ||
| DistributionModeHoldDownCommandClass::ClickedTargetAction(pSelect, currentAction, pTarget); | ||
| } | ||
| } | ||
| } | ||
| else | ||
| { | ||
| LEA_STACK(CellStruct* const, pCell, STACK_OFFSET(0x18, 0x8)); | ||
|
|
||
| auto invalidCell = CellStruct { -1, -1 }; | ||
| auto pSecondCell = action == Action::Move || action == Action::PatrolWaypoint || action == Action::NoMove ? pCell : &invalidCell; | ||
|
|
||
| for (const auto& pSelect : ObjectClass::CurrentObjects) | ||
| { | ||
| const auto currentAction = pSelect->MouseOverCell(pCell, false, false); | ||
|
|
||
| DistributionModeHoldDownCommandClass::ClickedCellAction(pSelect, currentAction, pCell, pSecondCell); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Unsorted::MoveFeedback = true; | ||
|
|
||
| return SkipGameCode; | ||
| } | ||
|
|
||
| DEFINE_HOOK(0x6DBE74, TacticalClass_DrawAllRadialIndicators_DrawDistributionRange, 0x7) | ||
| { | ||
| if (!DistributionModeHoldDownCommandClass::Enabled && SystemTimer::GetTime() - DistributionModeHoldDownCommandClass::ShowTime > 30) | ||
| return 0; | ||
|
|
||
| const auto spreadMode = Phobos::Config::DistributionSpreadMode; | ||
| const auto filterMode = Phobos::Config::DistributionFilterMode; | ||
|
|
||
| if (spreadMode || filterMode) | ||
| { | ||
| const auto pCell = MapClass::Instance.GetCellAt(DisplayClass::Instance.CurrentFoundation_CenterCell); | ||
| const auto color = (filterMode > 1) | ||
| ? ((filterMode == 3) ? ColorStruct { 255, 0, 0 } : ColorStruct { 200, 200, 0 }) | ||
| : ((filterMode == 1) ? ColorStruct { 0, 100, 255 } : ColorStruct { 0, 255, 50 }); | ||
| Game::DrawRadialIndicator(false, true, pCell->GetCoords(), color, static_cast<float>(spreadMode ? (2 << spreadMode) : 0.5), false, true); | ||
| } | ||
|
|
||
| return 0; | ||
| } |
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 think since this is so complex and spans across than a single "execute" method -- the actual distribution mode handling should not be a part of the command declaration/definition but elsewhere, and command classes should just call what they need in execute method. Same for hooks.
| const int range = (2 << spreadMode); | ||
| const auto center = pTechno->GetCoords(); | ||
| const auto pItems = Helpers::Alex::getCellSpreadItems(center, range); |
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.
You said that the code would need significant changes previously to accomodate arbitrary precise range, but I don't see what is significant here?
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.
Once we used shortcut keys because there were only a few available values. If there are too many available values, then the shortcut keys will become impractical and we need to delete them. There probably won't be much actual work, but there will be changes to the existing functions.
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 think there's a misunderstanding. I think shortcut keys could be left in with pre-set ranges, and dragging will mean an arbitrary range is selected. Both could exist in parallel.
Although if I am being honest I personally don't think there's a need in anything else other than drag-distribution. This is how it works in many modern RTS/RTT games with advanced controls and there's no customisation, I never heard it being a problem.
| Unsorted::MoveFeedback = false; | ||
| } | ||
|
|
||
| DEFINE_HOOK(0x4AE7B3, DisplayClass_ActiveClickWith_Iterate, 0x0) |
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 think this big hook should be split out into functions/methods called from a hook.

Reopen #1453 again.
Documents:
[ ]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 range.AllowDistributionCommand.SpreadMode&AllowDistributionCommand.FilterModeallow you to set spread range and target filter by hotkeys, which default toDefaultDistributionSpreadModeandDefaultDistributionFilterMode.AllowDistributionCommand.SpreadModeScrollset to true;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.AffectsEnemiesallow 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.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: