Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Basic handling of relative type-paths to procs #266

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

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