Skip to content

Conversation

@Leshana
Copy link
Contributor

@Leshana Leshana commented May 15, 2021

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)

@SpaceManiac
Copy link
Owner

SpaceManiac commented May 15, 2021

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?

@Leshana
Copy link
Contributor Author

Leshana commented May 16, 2021

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.

@SpaceManiac
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants