SpacemanDMM icon indicating copy to clipboard operation
SpacemanDMM copied to clipboard

Basic handling of relative type-paths to procs

Open Leshana opened this issue 3 years ago • 3 comments

Fixes false errors arising from relative paths to procs such as: error: failed to resolve path .HasProximity even when HasProximity is defined on the current type.

Details:

  • Adds a function that will attempt to resolve an identifer as a proc name if initial resolution fails; Returns NavigatePathResult so callers will know if it resolved to a proc or otherwise.
  • Use that function when resolving '.', '/', or ':' path operators in navigate(), as those operators can resolve to a proc reference and modify navigate_path to handle the case when navigate() resolves to a proc refernce.
  • Add unit tests covering common cases.
    • I drew test cases from the examples documented in SS13's callback.dm.

Example failure case:

/obj/machinery/camera/HasProximity(turf/T, atom/movable/AM, old_loc)
	// etc
/obj/machinery/camera/proc/upgradeMotion()
	sense_proximity(callback = .HasProximity)

Leshana avatar May 15 '21 19:05 Leshana

I don't consider being bug-compatible with DM a goal of SpacemanDMM. I consider .ProcName to be a bug-laden syntax because DM rejects these cases:

/atom/proc/callee()
/atom/proc/caller()
/atom/movable/caller()
	world << .callee
/atom/proc/callee()
/atom/movable/proc/caller()
	world << .callee
/atom/movable/proc/callee()
/atom/proc/caller()
/atom/movable/caller()
	world << .callee

If you're putting in the work to make SpacemanDMM a little closer to DM without compromising the structure of SpacemanDMM that's fine, but I'd rather get on the mark than overcorrect in the other direction (accepting code which DM rejects).

Also, is it worth encouraging (allowing?) coders to use .ProcName references when they might suddenly stop working if the peculiarities of which types override which procs (which otherwise doesn't matter) changes?

SpaceManiac avatar May 15 '21 19:05 SpaceManiac

Hmm, actually those are not correctly found as errors. In fact, it looks like detecting those would be quite difficult in the current SpacemanDMM architecture, as resolution depends on whether or not the lexically enclosing proc is a "proc" proc vs. an overridden proc. That information doesn't seem to be easily available.

Leshana avatar May 16 '21 00:05 Leshana

Sounds like that's probably not worth the complexity. The question then is whether to err on the side of generous or strict. My first instinct is to go for strict - not accepting .Foo syntax even though DM will sometimes (according to esoteric rules) accept it.

SpaceManiac avatar May 16 '21 06:05 SpaceManiac