Changeset View
Standalone View
source/simulation2/components/CCmpRangeManager.cpp
| Show First 20 Lines • Show All 146 Lines • ▼ Show 20 Lines | ||||||||||||||||||||||||||||||||||
| /** | /** | |||||||||||||||||||||||||||||||||
| * Computes the shared vision mask for the player | * Computes the shared vision mask for the player | |||||||||||||||||||||||||||||||||
| */ | */ | |||||||||||||||||||||||||||||||||
| static inline u16 CalcVisionSharingMask(player_id_t player) | static inline u16 CalcVisionSharingMask(player_id_t player) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| return 1 << (player-1); | return 1 << (player-1); | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| /** | /** | |||||||||||||||||||||||||||||||||
| * Checks whether v is in a parabolic range of (0,0,0) | * Checks whether v is in a parabolic range of (0,0,0) | |||||||||||||||||||||||||||||||||
Stan: Comments on top. | ||||||||||||||||||||||||||||||||||
| * The highest point of the paraboloid is (0,range/2,0) | * The highest point of the paraboloid is (0,range/2,0) | |||||||||||||||||||||||||||||||||
| * and the circle of distance 'range' around (0,0,0) on height y=0 is part of the paraboloid | * and the circle of distance 'range' around (0,0,0) on height y=0 is part of the paraboloid | |||||||||||||||||||||||||||||||||
| * | * | |||||||||||||||||||||||||||||||||
Not Done Inline ActionsI think you mean This equates (up to rounding errors) to (xx+yy)/2<= range(range/2-y) or equivalently xx+yy<= range(range-2y) Rounding errors, are slightly bad, but out of scope. bb: I think you mean `This equates (up to rounding errors) to (xx+yy)/2<= range(range/2-y) or… | ||||||||||||||||||||||||||||||||||
| * Avoids sqrting and overflowing. | * Avoids sqrting and overflowing. | |||||||||||||||||||||||||||||||||
| */ | */ | |||||||||||||||||||||||||||||||||
| static bool InParabolicRange(CFixedVector3D v, fixed range) | static bool InParabolicRange(CFixedVector3D v, fixed range) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| u64 xx = SQUARE_U64_FIXED(v.X); // xx <= 2^62 | u64 xx = SQUARE_U64_FIXED(v.X); // xx <= 2^62 | |||||||||||||||||||||||||||||||||
| u64 zz = SQUARE_U64_FIXED(v.Z); | u64 zz = SQUARE_U64_FIXED(v.Z); | |||||||||||||||||||||||||||||||||
| i64 d2 = (xx + zz) >> 1; // d2 <= 2^62 (no overflow) | i64 d2 = (xx + zz) >> 1; // d2 <= 2^62 (no overflow) | |||||||||||||||||||||||||||||||||
| ▲ Show 20 Lines • Show All 1,018 Lines • ▼ Show 20 Lines | void PerformQuery(const Query& q, std::vector<entity_id_t>& r, CFixedVector2D pos) | |||||||||||||||||||||||||||||||||
| else if (q.parabolic) | else if (q.parabolic) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| // elevationBonus is part of the 3D position, as the source is really that much heigher | // elevationBonus is part of the 3D position, as the source is really that much heigher | |||||||||||||||||||||||||||||||||
| CmpPtr<ICmpPosition> cmpSourcePosition(q.source); | CmpPtr<ICmpPosition> cmpSourcePosition(q.source); | |||||||||||||||||||||||||||||||||
| CFixedVector3D pos3d = cmpSourcePosition->GetPosition()+ | CFixedVector3D pos3d = cmpSourcePosition->GetPosition()+ | |||||||||||||||||||||||||||||||||
| CFixedVector3D(entity_pos_t::Zero(), q.elevationBonus, entity_pos_t::Zero()) ; | CFixedVector3D(entity_pos_t::Zero(), q.elevationBonus, entity_pos_t::Zero()) ; | |||||||||||||||||||||||||||||||||
| // Get a quick list of entities that are potentially in range, with a cutoff of 2*maxRange | // Get a quick list of entities that are potentially in range, with a cutoff of 2*maxRange | |||||||||||||||||||||||||||||||||
| m_SubdivisionResults.clear(); | m_SubdivisionResults.clear(); | |||||||||||||||||||||||||||||||||
| m_Subdivision.GetNear(m_SubdivisionResults, pos, q.maxRange * 2); | m_Subdivision.GetNear(m_SubdivisionResults, pos, q.maxRange * 2); | |||||||||||||||||||||||||||||||||
Not Done Inline Actionswe might need to twiddle a bit with these too. bb: we might need to twiddle a bit with these too. | ||||||||||||||||||||||||||||||||||
Done Inline ActionsI don't think we do actually, this accounts for entity size (by virtue of always returning one square too many). wraitii: I don't think we do actually, this accounts for entity size (by virtue of always returning one… | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsCheck the non-parabolic case too? Also note this will go wrong if size>maxRange or so bb: Check the non-parabolic case too? Also note this will go wrong if size>maxRange or so | ||||||||||||||||||||||||||||||||||
Done Inline ActionsNot sure what you mean? wraitii: Not sure what you mean? | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsThis will go wrong when r<h for some building, not changed by the patch out of scope bb: This will go wrong when `r<h` for some building, not changed by the patch out of scope | ||||||||||||||||||||||||||||||||||
| for (size_t i = 0; i < m_SubdivisionResults.size(); ++i) | for (size_t i = 0; i < m_SubdivisionResults.size(); ++i) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| EntityMap<EntityData>::const_iterator it = m_EntityData.find(m_SubdivisionResults[i]); | EntityMap<EntityData>::const_iterator it = m_EntityData.find(m_SubdivisionResults[i]); | |||||||||||||||||||||||||||||||||
| ENSURE(it != m_EntityData.end()); | ENSURE(it != m_EntityData.end()); | |||||||||||||||||||||||||||||||||
| if (!TestEntityQuery(q, it->first, it->second)) | if (!TestEntityQuery(q, it->first, it->second)) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| CmpPtr<ICmpPosition> cmpSecondPosition(GetSimContext(), m_SubdivisionResults[i]); | CmpPtr<ICmpPosition> cmpSecondPosition(GetSimContext(), m_SubdivisionResults[i]); | |||||||||||||||||||||||||||||||||
| if (!cmpSecondPosition || !cmpSecondPosition->IsInWorld()) | if (!cmpSecondPosition || !cmpSecondPosition->IsInWorld()) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| CFixedVector3D secondPosition = cmpSecondPosition->GetPosition(); | CFixedVector3D secondPosition = cmpSecondPosition->GetPosition(); | |||||||||||||||||||||||||||||||||
| // Restrict based on precise distance | // Restrict based on precise distance | |||||||||||||||||||||||||||||||||
| if (!InParabolicRange( | if (!InParabolicRange( | |||||||||||||||||||||||||||||||||
| CFixedVector3D(it->second.x, secondPosition.Y, it->second.z) | CFixedVector3D(it->second.x, secondPosition.Y, it->second.z) | |||||||||||||||||||||||||||||||||
| - pos3d, | - pos3d, | |||||||||||||||||||||||||||||||||
| q.maxRange)) | q.maxRange + fixed::FromInt(it->second.size))) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| if (!q.minRange.IsZero()) | if (!q.minRange.IsZero()) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| int distVsMin = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.minRange); | // Subtract 1 since we round up from the real size and we prefer returning too many entities. | |||||||||||||||||||||||||||||||||
| int distVsMin = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.minRange + fixed::FromInt(it->second.size - 1)); | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsWhy -1? Freagarach: Why `-1`? | ||||||||||||||||||||||||||||||||||
Done Inline ActionsSee comment below `// Subtract 1 since we round up from the real size and we prefer returning too many entities.` Basically we want to be too permissive. wraitii: See comment below ```// Subtract 1 since we round up from the real size and we prefer returning… | ||||||||||||||||||||||||||||||||||
Not Done Inline Actions
Please rerereredo your math again. also maybe inline bb: >Re-checked my maths, and I had a mistake for the min-range case.
Please rerereredo your math… | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsInstead of giving an intuitive argument like this (which surely can be made precise), one can also link to the proofs I made. bb: Instead of giving an intuitive argument like this (which surely can be made precise), one can… | ||||||||||||||||||||||||||||||||||
| if (distVsMin < 0) | if (distVsMin < 0) | |||||||||||||||||||||||||||||||||
Not Done Inline ActionsThese checks indeed seem to be valid, since we transform h-> h+2s and r->r+s/2, we now we check r^2+5rs+9/4s^2+2rh+hs>=d^2 which is a weaker check than r^2+2rh-d^2-s^2>=-2ds || d^2<=s^2 (use that 4r+r+h>=2r+r+h>=2sqrt(2r^2+2rh)>=2sqrt(2r^2+2rh)>=2d`, where we used the AG/MG inequality, if you don't see it I can write a proof for it). However (as one can see in this derivation) this bound is nowhere near sharp, and we can do much better by choosing different factors than 2 and .5 (do the same proof and try to find the best values, so that the least is cutoff, preferably only the AG/MG is used as inequality) bb: These checks indeed seem to be valid, since we transform h-> h+2s and r->r+s/2, we now we check… | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsNote that by AG/MG inequality we can also consider checking r^2+2rh+s^2+r+h>=d^2 (Nice exercise to show this follows from sqrt(r^2+2rh)>=d-s) bb: Note that by AG/MG inequality we can also consider checking ` r^2+2rh+s^2+r+h>=d^2` (Nice… | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsSorry missed a factor of s, it should be r^2+2rh+s^2+sr+sh>=d^2 (too bad one can't edit inline comments) bb: Sorry missed a factor of `s`, it should be `r^2+2rh+s^2+sr+sh>=d^2` (too bad one can't edit… | ||||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| r.push_back(it->first); | r.push_back(it->first); | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| std::sort(r.begin(), r.end()); | std::sort(r.begin(), r.end()); | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| // check a regular range (i.e. not the entire world, and not parabolic) | // check a regular range (i.e. not the entire world, and not parabolic) | |||||||||||||||||||||||||||||||||
| else | else | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| // Get a quick list of entities that are potentially in range | // Get a quick list of entities that are potentially in range | |||||||||||||||||||||||||||||||||
| m_SubdivisionResults.clear(); | m_SubdivisionResults.clear(); | |||||||||||||||||||||||||||||||||
| m_Subdivision.GetNear(m_SubdivisionResults, pos, q.maxRange); | m_Subdivision.GetNear(m_SubdivisionResults, pos, q.maxRange); | |||||||||||||||||||||||||||||||||
Not Done Inline ActionsAh, misunderstood these checks appear correct actually. bb: Ah, misunderstood these checks appear correct actually. | ||||||||||||||||||||||||||||||||||
| for (size_t i = 0; i < m_SubdivisionResults.size(); ++i) | for (size_t i = 0; i < m_SubdivisionResults.size(); ++i) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| EntityMap<EntityData>::const_iterator it = m_EntityData.find(m_SubdivisionResults[i]); | EntityMap<EntityData>::const_iterator it = m_EntityData.find(m_SubdivisionResults[i]); | |||||||||||||||||||||||||||||||||
| ENSURE(it != m_EntityData.end()); | ENSURE(it != m_EntityData.end()); | |||||||||||||||||||||||||||||||||
| if (!TestEntityQuery(q, it->first, it->second)) | if (!TestEntityQuery(q, it->first, it->second)) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| // Restrict based on precise distance | // Restrict based on precise distance | |||||||||||||||||||||||||||||||||
| int distVsMax = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.maxRange); | // Account for entity size assuming nearest-distance possible. | |||||||||||||||||||||||||||||||||
| int distVsMax = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.maxRange + fixed::FromInt(it->second.size)); | ||||||||||||||||||||||||||||||||||
Not Done Inline Actionsmaybe inline bb: maybe inline | ||||||||||||||||||||||||||||||||||
| if (distVsMax > 0) | if (distVsMax > 0) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| if (!q.minRange.IsZero()) | if (!q.minRange.IsZero()) | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| int distVsMin = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.minRange); | // Subtract 1 since we round up from the real size and we prefer returning too many entities. | |||||||||||||||||||||||||||||||||
| int distVsMin = (CFixedVector2D(it->second.x, it->second.z) - pos).CompareLength(q.minRange + fixed::FromInt(it->second.size-1)); | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsCan this become negative? Freagarach: Can this become negative?
(Also spaces.) | ||||||||||||||||||||||||||||||||||
Done Inline Actionsit->second.size is rounded up so it would have to be compared against 0 (maybe possible?) That being said, if it's negative it would also take a minRange of less than 1 to be relevant. Not sure it's actually a problem. wraitii: it->second.size is rounded up so it would have to be compared against 0 (maybe possible?)
That… | ||||||||||||||||||||||||||||||||||
bbUnsubmitted Not Done Inline Actionsspaces for consistancy bb: spaces for consistancy | ||||||||||||||||||||||||||||||||||
Not Done Inline Actionssame as L1222 bb: same as L1222 | ||||||||||||||||||||||||||||||||||
| if (distVsMin < 0) | if (distVsMin < 0) | |||||||||||||||||||||||||||||||||
| continue; | continue; | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
Not Done Inline ActionsSince here we actually check every entity and explicitly compute distances and stuff, we could be using the obstructionMan functions instead bb: Since here we actually check every entity and explicitly compute distances and stuff, we could… | ||||||||||||||||||||||||||||||||||
Done Inline ActionsI think it'd be too slow, but we can give it a shot. wraitii: I think it'd be too slow, but we can give it a shot. | ||||||||||||||||||||||||||||||||||
| r.push_back(it->first); | r.push_back(it->first); | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
Not Done Inline Actions
Do we need to create two CFixedVector2D each time ? Stan: Do we need to create two CFixedVector2D each time ?
You can check for the return value… | ||||||||||||||||||||||||||||||||||
Done Inline ActionsI'm fairly sure this all gets inlined and there's no construction of anything at all. wraitii: I'm fairly sure this all gets inlined and there's no construction of anything at all. | ||||||||||||||||||||||||||||||||||
| std::sort(r.begin(), r.end()); | std::sort(r.begin(), r.end()); | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| } | } | |||||||||||||||||||||||||||||||||
| virtual entity_pos_t GetElevationAdaptedRange(const CFixedVector3D& pos1, const CFixedVector3D& rot, entity_pos_t range, entity_pos_t elevationBonus, entity_pos_t angle) const | virtual entity_pos_t GetElevationAdaptedRange(const CFixedVector3D& pos1, const CFixedVector3D& rot, entity_pos_t range, entity_pos_t elevationBonus, entity_pos_t angle) const | |||||||||||||||||||||||||||||||||
| { | { | |||||||||||||||||||||||||||||||||
| entity_pos_t r = entity_pos_t::Zero(); | entity_pos_t r = entity_pos_t::Zero(); | |||||||||||||||||||||||||||||||||
| CFixedVector3D pos(pos1); | CFixedVector3D pos(pos1); | |||||||||||||||||||||||||||||||||
| ▲ Show 20 Lines • Show All 108 Lines • ▼ Show 20 Lines | Query ConstructQuery(entity_id_t source, | |||||||||||||||||||||||||||||||||
| Query q; | Query q; | |||||||||||||||||||||||||||||||||
| q.enabled = false; | q.enabled = false; | |||||||||||||||||||||||||||||||||
| q.parabolic = false; | q.parabolic = false; | |||||||||||||||||||||||||||||||||
| q.source = GetSimContext().GetComponentManager().LookupEntityHandle(source); | q.source = GetSimContext().GetComponentManager().LookupEntityHandle(source); | |||||||||||||||||||||||||||||||||
| q.minRange = minRange; | q.minRange = minRange; | |||||||||||||||||||||||||||||||||
| q.maxRange = maxRange; | q.maxRange = maxRange; | |||||||||||||||||||||||||||||||||
| q.elevationBonus = entity_pos_t::Zero(); | q.elevationBonus = entity_pos_t::Zero(); | |||||||||||||||||||||||||||||||||
| if (q.source.GetId() != INVALID_ENTITY) | ||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||
| EntityMap<EntityData>::const_iterator it = m_EntityData.find(q.source.GetId()); | ||||||||||||||||||||||||||||||||||
| ENSURE(it != m_EntityData.end()); | ||||||||||||||||||||||||||||||||||
| if (q.maxRange != entity_pos_t::FromInt(-1)) | ||||||||||||||||||||||||||||||||||
| q.maxRange += fixed::FromInt(it->second.size); | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsThe smallest size of an obstruction is known it is 0, so we can savely adjust the min range, by making it smaller. I propose to nuke this line bb: The smallest size of an obstruction is known it is `0`, so we can savely adjust the min range… | ||||||||||||||||||||||||||||||||||
| // It's fine if this goes negative. | ||||||||||||||||||||||||||||||||||
| if (q.minRange > entity_pos_t::Zero()) | ||||||||||||||||||||||||||||||||||
Not Done Inline Actionsif we prefer having "too many" ents shouldn't we do - here? bb: if we prefer having "too many" ents shouldn't we do `-` here? | ||||||||||||||||||||||||||||||||||
Done Inline ActionsAh, you're probably right yes wraitii: Ah, you're probably right yes | ||||||||||||||||||||||||||||||||||
| q.minRange -= fixed::FromInt(it->second.size); | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
Not Done Inline ActionsI am still right (see earlier inlines) bb: I am still right (see earlier inlines)
Errors will be thrown when building a fort otherwise | ||||||||||||||||||||||||||||||||||
Done Inline ActionsI don't think so, what I'm doing here is accounting for the querier size. So if you have a fort and you want a min-range of 1, you can discount units not at least '1+size-1' away. I do have to add, and I do take a minimal size addition. pre-Edit: however, the obstruction size is a diagonal as noted above so I actually would need to pick whichever side is smallest, but I don't have that data. wraitii: I don't think so, what I'm doing here is accounting for the querier size. So if you have a fort… | ||||||||||||||||||||||||||||||||||
| q.ownersMask = 0; | q.ownersMask = 0; | |||||||||||||||||||||||||||||||||
| for (size_t i = 0; i < owners.size(); ++i) | for (size_t i = 0; i < owners.size(); ++i) | |||||||||||||||||||||||||||||||||
| q.ownersMask |= CalcOwnerMask(owners[i]); | q.ownersMask |= CalcOwnerMask(owners[i]); | |||||||||||||||||||||||||||||||||
| if (q.ownersMask == 0) | if (q.ownersMask == 0) | |||||||||||||||||||||||||||||||||
| LOGWARNING("CCmpRangeManager: No owners in query for entity %u", source); | LOGWARNING("CCmpRangeManager: No owners in query for entity %u", source); | |||||||||||||||||||||||||||||||||
| q.interface = requiredInterface; | q.interface = requiredInterface; | |||||||||||||||||||||||||||||||||
| ▲ Show 20 Lines • Show All 1,078 Lines • Show Last 20 Lines | ||||||||||||||||||||||||||||||||||
Comments on top.