narc0tiq
New deposit overwrites old deposit with same label
A few times now, I have had a new marked ore deposit, use a label (NE 141, NW 351, etc) that an existing entry is already using.
The first time, they were the same type, and I just marked them both together to resolve that, but the next time, the deposits were different ore types. (3 actually, very close together)
Not sure how you're generating the label, but some sanity checking needs to be done to prevent this type of collision.
Agreed, thank you for making the issue explicit. #48 would help, but we definitely need to do something when the auto-generated name collides (even if it's as simple as popping up a dialog for the player to provide a different name).
I assume this would also fix the same problem that occurs with infinite ores like Angel's Refining adds, where patches outside the starter area have a mix of regular and infinite of the same type. Clicking the second overwrites the first in many cases.
Just adding this note in case this situation needs some special attention.
I'm guessing that the Right Thing is that two sites in the same octant that are equally distant from the origin want to each have their own entry, rather than one overwriting the other; is that sufficient, or would we want to give each site a distinct name?
I suspect that the simplest thing that could possibly work for a name collision is to name the new site 'site (1)', rather than 'site'.
Yes, any simple disambiguation will do, including suffixing the name. The big deal is really just to check that the site we're creating is actually distinct from the one we're conflicting with - if it's the same site, we should ignore it altogether.
The ore tracker may help - if each entity knew which sites it's a part of, it might simplify the conflict detection. I haven't given it much thought yet.
On Wed, Apr 24, 2019, 12:09 Martin Rudat [email protected] wrote:
I'm guessing that the Right Thing is that two sites in the same octant that are equally distant from the origin want to each have their own entry, rather than one overwriting the other; is that sufficient, or would we want to give each site a distinct name?
I suspect that the simplest thing that could possibly work for a name collision is to name the new site 'site (1)', rather than 'site'.
— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/narc0tiq/YARM/issues/52#issuecomment-486136679, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEQNVMT5HYECQM3O5HAV5LPSAPS7ANCNFSM4CVZ75FA .
We could maintain a chunk <-> site index.
Using such an index, if the new site shares a chunk with an existing site with the same ore type, we could instead extend the old site.
If we want to retain support for distinct sites for two discontiguous ore patches that happen to share a chunk, we could have chunk-based index of resource entities within a site, and do an exhaustive comparison within the overlapping chunks for when a new site shares the same entity type and at least one chunk with an existing site, to see if the new site is contiguous with the old site.
If each resource entity has a valid unit_number, we could directly index by that, but I suspect that they do not have such an identifier.
On a not entirely unrelated topic, I suspect that for expanding a site, it should be faster to expand a chunk at a time (or at at some grid larger than a single tile), rather than a tile at a time, given that Lua <-> C is relatively expensive, as searching for all of the ore entities in a chunk as a single call should be in total faster (if not as awesome looking) than searching a tile at a time. For a grid to search by if a chunk at a time is too much, a grid 2, 3 or 5 tiles to a side probably makes the most sense.
Edit: it should be entirely feasible to maintain an index of position -> entity, but I'm not sure it's worth doing that.
Edit the second: On further thought, [chunk coordinates (as a string)][tile coordinates within chunk] would be an entirely feasible index structure to maintain.
If each resource entity has a valid unit_number, we could directly index by that, but I suspect that they do not have such an identifier.
Correct, resources are not units.
We could maintain a chunk <-> site index.
I... don't know if that makes sense, because I'm having a hard time following your plan.
Let me restate the problem from the start:
Sites are uniquely* identified by their name, therefore if a site gets a name that had already been given to another site, we have a conflict, and one of the sites will overwrite the other (the new site replaces the old site -- without performing any cleanup!).
There are exactly two cases when a site gets a new name assignment:
- when it is created or expanded (the same code runs for both cases), or
- when it is renamed.
In both cases, it is possible that the new name is already assigned to a conflicting site, in which case we need to check:
- is the conflicting site actually the same site? If so, we don't need to do anything at all!
- otherwise:
- if this is the player renaming a site, show them an error and refuse to do the operation
- else, try to disambiguate the name (e.g., by suffixing
#2) and repeat the conflict check, incrementing the number after the#until you reach a valid new name.
Currently, there are no checks done at all -- you can replace any site with any other just by renaming it, and you can be unlucky and end up with two distinct sites that get the same auto-generated name.
Specific checks that can tell whether two sites are the same:
- Are they literally the same table object?
- Do they have the same ore type?
- Do they have the same ore entities?
I'm guessing the very first of the above checks would suffice in almost every case where we would have to run them, but I want to have the option of running the other checks, too. Note that for number 3, it suffices to check whether they have the same ore tracker indices (and finding any ore in one site that isn't in the other is enough to guarantee the sites are different).
Footnote:
* - uniqueness is within a force -- two forces can have sites with the same name, but these will never conflict with each other.
I suspect that for expanding a site, it should be faster to expand a chunk at a time (or at at some grid larger than a single tile)
On the one hand, I completely agree -- I suspect even a 128x128 tile grid would be perfectly timely. On the other hand, site expansion is such a rare activity that I really don't care about optimizing it. It's already rate-limited to 30 entities per player per tick, and the code is relatively easy to follow (as long as you're aware of the on_tick).
Plus, site detection looks pretty.
Also, re:
an index of position -> entity
Please take a look at the ore tracker, which does contain one such index (with an extra layer of indirection).