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

Doc: Revive old-simulator-doc #5439

Closed
wants to merge 33 commits into from
Closed

Conversation

Slayer95
Copy link
Contributor

simulator-doc.txt lacks the detailed explanations from old-simulator-doc.txt.

This PR intends to bring the old documentation file up to date, so that it can be a reliable source, and in the future be merged into the main simulator-doc.txt file.

simulator-doc-extended.txt Outdated Show resolved Hide resolved
@scheibo
Copy link
Contributor

scheibo commented Apr 12, 2019

Ahah! Great minds think alike! This was on my TODO list as well! I'd like the merge them and update both.

simulator-doc-extended.txt Outdated Show resolved Hide resolved
simulator-doc-extended.txt Outdated Show resolved Hide resolved
simulator-doc-extended.txt Outdated Show resolved Hide resolved
simulator-doc-extended.txt Outdated Show resolved Hide resolved
simulator-doc-extended.txt Outdated Show resolved Hide resolved
@Slayer95 Slayer95 requested a review from Zarel April 12, 2019 05:07
@Zarel
Copy link
Member

Zarel commented Apr 12, 2019

I assume this isn't done yet? I wrote these docs at some point but they always struck me as a confusing outdated mess; it's honestly easier to find events by reading the actual source code, for me.

@Zarel
Copy link
Member

Zarel commented Apr 12, 2019

I can't imagine they'd ever be kept up to date even if we did reintroduce the docs, but I mean if you want to try, be my guest.

@Slayer95
Copy link
Contributor Author

While it's certainly and humongous task to keep this fully updated, there are some things we can do.

  1. We can comprehensively document non-move single events.
  2. We can use this document to introduce the PS event system to new devs.

So what happens with move single events, and with the global events?

  • singleEvents run on moves are currently expected to be documented in simulator-doc.txt (ofc the diagram is absolutely outdated after the latest spread move refactor).
  • Global events' signatures are comprehensively documented in dev-tools/globals.ts, as noted in this PR.

Regarding this PR, I expect comments about precision, style, and the like, for both goals 1 and 2. I am still missing some events to fully accomplish goal 1: onCopy for statuses, and all except onStart for items and abilities.

@Slayer95
Copy link
Contributor Author

@scheibo
Copy link
Contributor

scheibo commented Apr 12, 2019

I can't imagine they'd ever be kept up to date even if we did reintroduce the docs, but I mean if you want to try, be my guest.

If you're motivated, keeping docs (relatively) up to date isn't that hard. Remembering during review to check if the changes would affect the docs is the best way, but this doesn't catch everything (though we could set up a PR template to remind contributors of this). At work, on Mondays I commit to looking over the previous week's worth of changes and checking to see if any would require my documentation to be updated and then updating the documentation all at once, meaning my project's docs are never more than ~1 week out of date.

I think the best way to avoid docs becoming out of date is to document the right things - ie/ high level stuff which doesn't change as frequently. While method signatures etc might subtly change, high level concepts like how the event system works etc are less likely to become obsolete as quickly (though admittedly, I've proposed reworking the event system, but I'd be more than happy to update the docs for that).

@Zarel
Copy link
Member

Zarel commented Apr 12, 2019

If you want to like auto-render parts of this file from dev-tools/globals.ts, maybe even require comments, that might be nice for keeping things up to date.

It might also be hell. Like I said in a different issue, an event registration syntax might be better than the current pure-declarative one.

@Slayer95
Copy link
Contributor Author

It might also be hell. Like I said in a different issue, an event registration syntax might be better than the current pure-declarative one.

While the future event registration API hasn't been designed yet, I think it would be in our best interest to keep (most of) the current event names and signatures

If you want to like auto-render parts of this file from dev-tools/globals.ts, maybe even require comments, that might be nice for keeping things up to date.

I don't think adding a build step for the documentation is within scope, but that can be done in the future.

SIM_EVENTS.md Outdated
Event propagation is the mechanism by which multiple effects have their event handlers
called when a global event is run. They are called in order, from higher to lower priority,
as per their listed Priority values. The priority should be specified with the key
``${handlerName}Priority``, where `handlerName` is the full name of the associated event handler.
Copy link
Contributor Author

@Slayer95 Slayer95 Apr 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

`${handlerName}Priority`

wow, got it!

@Iyarita
Copy link

Iyarita commented Jul 1, 2019

I just updated this on another request. I didn't realize it had already been worked on. Sorry.

@AnnikaCodes
Copy link
Collaborator

No activity for 3 years—closing this.

@Slayer95
Copy link
Contributor Author

There is some stuff that has gone outdated, but this is mostly still relevant, and above all, useful.

Reopen for visibility, please.

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.

5 participants