Adding documentation and smoothing out formatting for non-UI code. by BeowulfDragon · Pull Request #10 · EmilyV99/GodotAP

BeowulfDragon · 2026-05-11T17:03:07Z

I was fiddling around with something and wanted to use this library, but found it hard to understand and read, so I have added a bunch of documentation and made the formatting more consistent and readable. I mainly followed the recommended GDScript style guide, except where an alternative style was consistently used and readable. I also broke up as many lines that went over 100 characters as I could, excluding some long strings.

This PR doesn't include documentation for the main UI and app code, only for the files in the ap_files, autoloads, and managers directories, and save_file.gd. Any non-documentation changes should have no functional differences, only changes to formatting.

No AI was used in the work done here, I'm just like this.

Tweaking the documentation of archipelago.gd and adding/changing spacing and indentation (per the GDScript style guide suggested in the Godot docs) to make it less of a wall of code.

ap_location.gd also has a super broken call, but I'm exclusively doing documentation and formatting currently.

Finished writing documentation for all the classes in ap_files. Also fixed some stuff in archipelago.gd. Finished the documentation of command_manager.gd too. Also I was wrong in a previous commit message about an error in APLocation, I just didn't notice the conn member ~300 lines deep into the source for AP.

With an asterisk, since there's a lot of UI in the domain code, so there has been documentation of UI code.

Still need to do one last big read through before submitting a PR.

Making continuation lines more consistent, enforcing spacing around operators, and more spacing in single-line dictionaries.

Also added documentation to SaveFile, as the docs for APSaveManager wouldn't generate without that having documentation to generate.

EmilyV99

did not review archipelago.gd, command_manager.gd, console_command.gd, or connection_info.gd yet (my brain is not prepared for those files atm).

Also didn't actually look at the generated docs in Godot from this yet.

Gonna drop this with the review comments I have so far though, will need to look deeper later (and probably remove the trackerpack stuff from config.gd on my end, which you noticed wasn't being used at all, as it was entirely outdated 😅)

Feel free to bump this in the discord thread if I don't look at it further soon and you have everything I've commented on here cleaned up

Co-authored-by: Emily <35015090+EmilyV99@users.noreply.github.com>

For Network[Thing]s, Create -> Deserialize. While it's maybe technically demarshalling rather than deserialization, deserialization gets the point across enough. Also not using "create" makes it clear that nothing is happening on the server or other clients.

EmilyV99

looked through the command code, still gotta check the two big main files.

EmilyV99 · 2026-05-25T16:51:39Z

+# Commands currently available to use, indexed by name.
 var _commands_by_name: Dictionary[String, ConsoleCommand]
+
+## Controls if debug commands should be hidden.


Suggested change

## Controls if debug commands should be hidden.

## Controls if debug commands should be hidden.

## Cannot be modified in non-debug builds.

EmilyV99 · 2026-05-25T16:57:07Z

+
+
+## Get the autofilled parameters for a command message, up to the number of arguments specified in
+## [param capacity].


Suggested change

## [param capacity].

## [param capacity]. A capacity of 0 is unlimited.

EmilyV99 · 2026-05-25T17:00:01Z

+		return
 	var cmd := get_command(msg.split(" ", true, 1)[0])
 	if cmd and cmd.call_proc:
 		if cmd.is_disabled():


Suggested change

if cmd.is_disabled():

if cmd.is_disabled() or (cmd.is_debug() and debug_disabled()):

just noticed this issue on my end 😅

EmilyV99 · 2026-05-25T17:01:59Z

+
+# Check if [param cmd] is enabled.
 static func _cmd_is_enabled(cmd: ConsoleCommand) -> bool:
 	return not cmd.is_disabled()


Suggested change

return not cmd.is_disabled()

return not (cmd.is_disabled() or (cmd.is_debug() and debug_disabled()))

same error on my end from above 😅

EmilyV99 · 2026-05-25T17:09:08Z

+
+## Set [member autofill_proc].
 func set_autofill(caller: Variant) -> ConsoleCommand:
 	assert(caller is bool or caller is Callable)


Suggested change

assert(caller is bool or caller is Callable)

assert(caller is Callable)

caller being a bool here seems to be an error . . . huh. Another me issue 😅

EmilyV99

finally done reviewing 😅 lmk if you have any follow-up questions on anything I commented on

EmilyV99 · 2026-05-25T17:20:40Z

+var serv_version: Version 
+## The generator's Archipelago version.
+var gen_version: Version 
+## The seed name received from the server.


Suggested change

## The seed name received from the server.

## The seed_name received from the server.

seed_name is the specific key the server sends, naming it the same in the comment here was intentional

EmilyV99 · 2026-05-25T17:21:51Z

+var seed_name: String 
+## The ID of your player.
+var player_id: int 
+## The ID of your team (unimplemented).


Suggested change

## The ID of your team (unimplemented).

## The ID of your team.

teams are being implemented soon™ re: Silvris' teams PR, probably correct to remove this note now

EmilyV99 · 2026-05-25T17:22:19Z

+var player_id: int 
+## The ID of your team (unimplemented).
+var team_id: int 
+## The slot data from the server.


Suggested change

## The slot data from the server.

## The slot_data from the server.

same reasoning as seed_name

EmilyV99 · 2026-05-25T17:23:41Z

 		return hints
-
-## All locations, by ID
+## All locations, by ID.


Suggested change

## All locations, by ID.

## All locations, by ID.

extra line after getter/setter seems more correct to me for readability?

EmilyV99 · 2026-05-25T17:40:12Z

+## Emitted for each item [i]packet[/i] received.
 signal obtained_items(items: Array[NetworkItem])
-## Emitted when the server re-sends ALL obtained items
+## Emitted when the server re-sends ALL obtained items.


Suggested change

## Emitted when the server re-sends ALL obtained items.

## Emitted when the server re-sends ALL obtained items, after

## [signal obtained_items] fires.

EmilyV99 · 2026-05-25T17:54:27Z

-	## The node is needed, as the representation can depend on the node's 'Theme'.
+
+
+	## Get the color represented by this complex color for the proviced [param node].


Suggested change

## Get the color represented by this complex color for the proviced [param node].

## Get the color represented by this complex color for the provided [param node].

EmilyV99 · 2026-05-25T18:00:17Z

-## Gets a RichColor from a string. 'RichColor.NIL' is returned if the string is invalid.
+
+
+## Gets a [enum RichColor] from [param s]. [code]NIL[/code] is returned if the string is invalid.


Suggested change

## Gets a [enum RichColor] from [param s]. [code]NIL[/code] is returned if the string is invalid.

## Gets a [enum RichColor] from [param s]. [member RichColor.NIL] is returned if the string is invalid.

this should be a reference to the specific member, shouldn't it?

EmilyV99 · 2026-05-25T18:08:21Z

+## Null unless a node inheriting from [APSaveManager] is added to to 
+## [code]godot_ap/autoloads/archipelago.tscn[/code].
+## [br][br]
+## Can be [code]null[/code] if not provided in [code]godot_ap/autoloads/archipelago.tscn[/code].


Huh. The problem was present before, but this appears to be just saying the same thing twice in a row redundantly. Should reword to not repeat itself

EmilyV99 · 2026-05-25T18:39:25Z

-## Changes this connection's DeathLink group
-## Will only send/receive deaths with other clients in the same group
+
+## Changes this connection's DeathLink group.


Suggested change

## Changes this connection's DeathLink group.

## Changes this connection's DeathLink group name.

match comment on function below

EmilyV99 · 2026-05-25T18:40:40Z

+
 ## Set the current Archipelago status.
-## Set to 'CLIENT_GOAL' when the player has 'won'.
+## Set to [code]CLIENT_GOAL[/code] when the player has won.


could this reference ClientStatus.CLIENT_GOAL with some syntax, rather than a generic code tag?

BeowulfDragon added 8 commits May 7, 2026 16:59

Archipelago.gd documentation and style

3400ee0

Tweaking the documentation of archipelago.gd and adding/changing spacing and indentation (per the GDScript style guide suggested in the Godot docs) to make it less of a wall of code.

Documentation work on connection_info.gd

d6d52ec

More documentation and formatting.

d61982c

ap_location.gd also has a super broken call, but I'm exclusively doing documentation and formatting currently.

Last non-UI documentation.

963d6e7

With an asterisk, since there's a lot of UI in the domain code, so there has been documentation of UI code.

A couple more minor touchups.

e8a7819

Still need to do one last big read through before submitting a PR.

Another readability pass.

77973c8

Making continuation lines more consistent, enforcing spacing around operators, and more spacing in single-line dictionaries.

Final couple documentation formatting fixes.

3d4ab7a

Also added documentation to SaveFile, as the docs for APSaveManager wouldn't generate without that having documentation to generate.

EmilyV99 requested changes May 12, 2026

View reviewed changes

BeowulfDragon and others added 4 commits May 12, 2026 14:39

Apply suggestions from code review

99407d6

Co-authored-by: Emily <35015090+EmilyV99@users.noreply.github.com>

Adding extension documentation to APSaveManager

47c072e

Merge branch 'main' into main

a5b9f55

EmilyV99 requested changes May 25, 2026

View reviewed changes

	## Controls if debug commands should be hidden.
	## Controls if debug commands should be hidden.
	## Cannot be modified in non-debug builds.



		## Get the autofilled parameters for a command message, up to the number of arguments specified in
		## [param capacity].

	## [param capacity].
	## [param capacity]. A capacity of 0 is unlimited.

	if cmd.is_disabled():
	if cmd.is_disabled() or (cmd.is_debug() and debug_disabled()):

	return not cmd.is_disabled()
	return not (cmd.is_disabled() or (cmd.is_debug() and debug_disabled()))

	assert(caller is bool or caller is Callable)
	assert(caller is Callable)

	## The seed name received from the server.
	## The seed_name received from the server.

	## The ID of your team (unimplemented).
	## The ID of your team.

	## The slot data from the server.
	## The slot_data from the server.

	## Emitted when the server re-sends ALL obtained items.
	## Emitted when the server re-sends ALL obtained items, after
	## [signal obtained_items] fires.

		## The node is needed, as the representation can depend on the node's 'Theme'.


		## Get the color represented by this complex color for the proviced [param node].

		## Gets a RichColor from a string. 'RichColor.NIL' is returned if the string is invalid.


		## Gets a [enum RichColor] from [param s]. [code]NIL[/code] is returned if the string is invalid.

	## Changes this connection's DeathLink group.
	## Changes this connection's DeathLink group name.

Conversation

BeowulfDragon commented May 11, 2026

Uh oh!

EmilyV99 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EmilyV99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EmilyV99 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants