BeeStation-Hornet icon indicating copy to clipboard operation
BeeStation-Hornet copied to clipboard

Some `FAST_REF(thing)` or `REF(thing)` should be replaced to refer its weakref datum, like `FAST_WEAKREF(thing)`

Open EvilDragonfiend opened this issue 10 months ago • 10 comments

Describe the feature request

Some REF(thing) should be replaced to refer its weakref datum - kinda FAST_WEAKREF(thing)

Let's say there is a John /mob[0x0001], and we know its ref value once John /mob[0x0001] is deleted, a memory slot 0x0001 becomes free, be allowed to have a new mob. and then Janne /mob[0x0001] will take the memory slot.

Meanwhile, if something was logged as my_friend = "[0x0001]", it refers to Janne because the mob takes the memory, but real my_friend is supposed to be John.

Thus, there should be a kind of proc that takes a reference value of a type's weakref. created weakref for Jobn will be /datum/weakref [0x900001], and it will be my_friend = "[0x900001]" Even if John is deleted, weakref still exists. Even if weakref refers to [0x0001], its okay. We used a key as John's weakref, and we don't have to resolve it.

If a weakref for Janne is created, it will be /datum/weakref [0x900002], my_friend = "[0x900001]" still refers to John (exactly, John's weakref). thus it solves a bad ref issues.

#define FAST_WEAKREF(thing) FAST_REF(WEAKREF(thing))

FAST_WEAKREF should be that form.

EvilDragonfiend avatar Apr 15 '24 09:04 EvilDragonfiend

Labeling Bug too since it's potentially buggy.

EvilDragonfiend avatar Apr 15 '24 09:04 EvilDragonfiend

Example) image

EvilDragonfiend avatar Apr 15 '24 09:04 EvilDragonfiend

Is \ref not unique?

The obvious solution when implementing a game engine is to just increment a counter for refs, or use a UUID. It would take a lot of extra effort with little gain to have a game engine re-use deleted refs.

PowerfulBacon avatar Apr 15 '24 09:04 PowerfulBacon

Is \ref not unique?

Not trully unique because qdeleting releases the memory slot, and new mob will take the slot.

The obvious solution when implementing a game engine is to just increment a counter for refs, or use a UUID. It would take a lot of extra effort with little gain to have a game engine re-use deleted refs.

maaaaybeee? /datum/var/static/counter and /New() { counter++ }??

EvilDragonfiend avatar Apr 15 '24 09:04 EvilDragonfiend

/datum/proc/unique_ref_count()
    if(!ref_counter)
        ref_counter = ++global_counter
    return ref_counter

/datum
    var/static/global_counter
    var/ref_counter

Maybe this?

EvilDragonfiend avatar Apr 15 '24 09:04 EvilDragonfiend

No, thats not what I mean. I was talking about byond-level implementation details, not something codeable in dreammaker

PowerfulBacon avatar Apr 15 '24 10:04 PowerfulBacon

Not trully unique because qdeleting releases the memory slot, and new mob will take the slot.

I guess, this highly depends on the implementation since byond is going to be using its own virtual memory implementation and not physical addressing

PowerfulBacon avatar Apr 15 '24 10:04 PowerfulBacon

Not trully unique because qdeleting releases the memory slot, and new mob will take the slot.

I guess, this highly depends on the implementation since byond is going to be using its own virtual memory implementation and not physical addressing

That's why this issue says about using weakref as a type's true unique ref id, because we don't qdel weakref

image

/proc/main()
  var/datum/D = new()
  D.tag = "first one"
  world.log << "< First datum >"
  world.log << "My tag: [D.tag]"
  var/reftext = ref(D)
  world.log << "My ref: [reftext]"
  del(D)
  D = new()

  world.log << ""
  var/datum/another_datum = locate(reftext)
  world.log << "< Second datum >"
  world.log << "My tag: [another_datum.tag]"
  world.log << "My ref: [ref(another_datum)]"
  world.log << "First datum ref: [reftext]"

As the example I described, It has a pottential issue with it.

EvilDragonfiend avatar Apr 15 '24 10:04 EvilDragonfiend

I see, this may be an issue. Using a weakref wrapper around all objects is a big cost though, if this doesn't cause any issues in practice then it might be worth trying to find issues first.

PowerfulBacon avatar Apr 15 '24 10:04 PowerfulBacon

I see, this may be an issue. Using a weakref wrapper around all objects is a big cost though, if this doesn't cause any issues in practice then it might be worth trying to find issues first.

Not all objects need to do that. For now, a visible first issue is

image

when a mob is deleted and then a new mob is created, some mobs can have them allied even if that mob isn't actually what their ally - as the issue described. and I wonder which cost you're concerning on as which aspect.

If you think it'll be a little bit slow to access, I think it won't be that slow...

#define FAST_WEAKREF(thing) (thing.weak_reference?.cached_ref || FAST_REF(WEAKREF(thing)))

Hopefully?

EvilDragonfiend avatar Apr 15 '24 10:04 EvilDragonfiend