Fix/jumpgate load by Zalatos · Pull Request #189 · TheGiddyLimit/betterR20

Zalatos · 2025-04-01T21:00:21Z

prevents some errors and enables some features.

attempted workaround to get canvas context
jukebox playlist error handled
fix: character sheet not found when on jumpgate

TheGiddyLimit

in addition to the other feedback, please avoid pushing the dist/ ... changes

you can either rebase/amend your commits to remove those changes, make a new set of commits without committing the dist/ files, or I can just copy out the rest of the patch if/when the PR is otherwise ready

TheGiddyLimit · 2025-04-06T21:14:00Z

 		{name: "bestiary index", url: `${MONSTER_DATA_DIR}index.json`},
 		{name: "bestiary fluff index", url: `${MONSTER_DATA_DIR}fluff-index.json`},
 		{name: "bestiary metadata", url: `${MONSTER_DATA_DIR}legendarygroups.json`},
-		{name: "adventures index", url: `${DATA_URL}adventures.json`},


this file definitely should exist (see https://5e.tools/data/adventures.json); what errors are you seeing?

yeah that endpoint responded with 404

it's definitely alive and well for me, can you double-check it in your browser?

works in browser .

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://5e.tools/data/adventures.json. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 200.

can run this in the console while in a roll20 game
var x = await fetch("https://5e.tools/data/adventures.json")
will give same result

👍 have asked the server host to take a look at it, because it really should be available, but it's behind some CloudFlare thing

TheGiddyLimit · 2025-04-06T21:14:27Z

 		d20plus.sheet = "ogl";
-		if (window.is_gm && (!d20.journal.customSheets || !d20.journal.customSheets)) {
+		const sheets = d20.journal.customSheets ?? d20.journal.characterSheetsManager.getAllSheets();
+		if (window.is_gm && sheets.length < 1) {


sheets.length < 1 -> !sheets.length

nah, wanted to avoid the concept where length property did not exist. this clearly denotes its expecting an array since the jumpgate now stores sheets in an array (roll20 supports more than 1 sheet type for campaign now)
can make it cleaner though. as long as it handles both jumpgate and non jumpgate games.

i'm not too attached to format

certainly isn't clear enough for me to understand what's going on, so, would appreciate a revision to make it more explicit; you're saying you're relying on undefined < 1 === false..?

if d20.journal.customSheets and d20.journal.characterSheetsManager.getAllSheets() are two fundamentally different data types, then there should just be two different branches, one to handle each. I.e., I'd prefer duplicated code to confusing checks at every usage point

i see. yes, they are 2 fundamentally different ways to provide sheets. 1 is an array 1 is 1 item in a non existent array

TheGiddyLimit · 2025-04-06T21:18:57Z

+		if (firstSheet.layouthtml.indexOf("shaped_d20") > 0) d20plus.sheet = "shaped";
+		if (firstSheet.layouthtml.indexOf("DnD5e_Character_Sheet") > 0) d20plus.sheet = "community";


while you're here,

firstSheet.layouthtml.indexOf("shaped_d20") > 0 -> firstSheet.layouthtml.includes("shaped_d20")

firstSheet.layouthtml.indexOf("DnD5e_Character_Sheet") > 0 -> firstSheet.layouthtml.includes("DnD5e_Character_Sheet")

sweet i like it

TheGiddyLimit · 2025-04-06T21:19:59Z

-		$iptUrl.data("id", "lmop");
-		const $sel = $("#button-adventures-select");
-		adventureMetadata.adventure.forEach(a => {
+		if (adventureMetadata.adventure){


this smells like a hack due to disabling the adventure index load..? Or is there some other purpose

if u want to call it a hack. if there is no result from the adventures list it will still cause the rest of the load to break when other features are still functional which sounds unnecessary

TheGiddyLimit · 2025-04-06T21:21:45Z

-		d20.engine.canvas.sortTokens = _.bind(d20plus.mod.sortTokens, d20.engine.canvas);
-		d20.engine.canvas.drawAnyLayer = _.bind(d20plus.mod.drawAnyLayer, d20.engine.canvas);
-		d20.engine.canvas.drawTokensWithoutAuras = _.bind(d20plus.mod.drawTokensWithoutAuras, d20.engine.canvas);
+		if (d20.engine.canvas){


nit: missing between ){

TheGiddyLimit · 2025-04-06T21:21:58Z

 			cacheRenderLoop();
 		};

+		if (!d20.engine.canvas.on) {


repeated from above? Is this something that could be factored out, and given e.g. a useful function name explaining what it's doing?

additionally, what is $.on/$.off doing? Are these ever defined?

so $.off and on are query event listener hooks.
the repeat is because i was not certain with 100% certainty where the first call to the d20.enginge.canvas was.
so wit this i created the functions to apply the event listeners.

was too lazy to rely on element.addEventListener with the custom events that were expected to exist here so relied on jquery.

i'm not even sure if his is a full fix.
there are still things i want to fix later

the jQuery bodge I'm ultimately fine with; it's a hack, but it's a fairly elegant (and easy to un-hack later) one, so that's all good

$.on/$.off don't exist in my test game, hence the Q:

i think i had a backup in there . like pulling the ref to the relevant canvas and stuff. i think i tried to prototype it too. but havent been able to get it working

revised. found the location for them.$.fn.on/off

TheGiddyLimit · 2025-04-06T21:24:20Z

-			d20plus.engine.forwardOneLayer(n);
-			return false;
-		});
+		if (typeof(Mousetrap) !== "undefined"){


nit typeof(Mousetrap) -> typeof Mousetrap

TheGiddyLimit · 2025-04-06T21:25:26Z

 				.then(response => response.json())
 				.then(data => {
+					// check if blocked
+					if (data.block)


nit: move non-block-statement if statement onto same line as condition (or wrap in { })

ok so u prefer single like non-block if statements?
like this?
if(conditionIsMet) doTheThing();

missing a space, but yes

if (conditionIsMet) doTheThing();

TheGiddyLimit · 2025-04-06T21:26:02Z

 					if ((typeof window.d20 !== "undefined" || window.currentPlayer?.d20) && !$("#loading-overlay").is(":visible") && !hasRunInit) {
 						hasRunInit = true;
 						if (!window.d20) window.d20 = window.currentPlayer.d20;
+						d20.engine.canvas ??= $("#babylonCanvas").get(0);


why not document.getElementById(...) and skip the jQuery cruft?

no reason. if jquery had to be loaded before this was loaded thought i'd use it. less characters.
if u want getElementById can change for better portability. not attached

in the case where it's jQuery selector -> immediately de-reference it to underlying DOM element (with .get(0) or [0]), then prefer using the browser built-in

character count is less important than flipping between two mental contexts for the sake of half a line

TheGiddyLimit · 2025-04-06T21:29:32Z

appreciate the submission generally, but, hard to judge a set of changes without information on the intent/effect of those changes

screenshots of things working (or even better, "before" screenshot of things not working, and "after" screenshots of the working version with fixes applied) would be greatly appreciated

Zalatos · 2025-04-07T07:32:04Z

in addition to the other feedback, please avoid pushing the dist/ ... changes

you can either rebase/amend your commits to remove those changes, make a new set of commits without committing the dist/ files, or I can just copy out the rest of the patch if/when the PR is otherwise ready

sorry about that. i think the editorconfig overwrote the dist folder rollback on my amends. saving the file added the spaces etc.
was getting annoyed trying to fix it and left it for another time

Zalatos · 2025-04-07T07:49:46Z

in addition to the other feedback, please avoid pushing the dist/ ... changes
you can either rebase/amend your commits to remove those changes, make a new set of commits without committing the dist/ files, or I can just copy out the rest of the patch if/when the PR is otherwise ready

sorry about that. i think the editorconfig overwrote the dist folder rollback on my amends. saving the file added the spaces etc. was getting annoyed trying to fix it and left it for another time

no worries. since the live version is completely broken i thought i'd share my progress on fixing some of the functionality.
apparently VTTES is also broken so my next journey is that before i come back here

Zalatos · 2025-04-07T07:56:53Z

next update:

will try reverse the dist file content by saving without an editorconfig.
will 1 liner the non-block if statement
add getElementById instead of jquery version
change typeof() to typeof shorthand
change array.indeoxOf to array.includes

into fix/jumpgate-load

… in jumpgate

Zalatos · 2025-04-08T08:48:59Z

should dist folder be added to gitignore?

TheGiddyLimit · 2025-04-14T11:05:06Z

should dist folder be added to gitignore?

not currently, as the compiled scripts are committed when a new release is put out

Zalatos added 3 commits March 25, 2025 17:04

fix(jumpgate load): allows jumpgate to load the main parts of the script

7d63140

fix: syntax error

32ca746

fix: keep compatible with legacy games

c4b2558

TheGiddyLimit requested changes Apr 6, 2025

View reviewed changes

Zalatos added 2 commits April 7, 2025 18:49

fix: keep compatible with legacy games

0e301be

Merge branch 'fix/jumpgate-load' of https://github.com/Zalatos/betterR20

e1c00df

into fix/jumpgate-load

Zalatos marked this pull request as draft April 7, 2025 17:31

Zalatos added 3 commits April 7, 2025 19:36

refactor: changed some format styles based on feedback

9981b08

docs: added some coding config based on feedback

56c9541

refactor: change basic id calls to use getElementById

de4cf08

Zalatos requested a review from TheGiddyLimit April 7, 2025 20:10

Zalatos added 3 commits April 7, 2025 22:21

fix: canvas .prototype is undefined error

df2f559

fix(JumpGate): Load errors but disable a few features that dont apply…

98b48d4

… in jumpgate

refactor: added missing feedback

43d29b5

Zalatos added 2 commits April 16, 2025 14:41

fix: stuck on load without error

0f6c2fc

fix: fix hang in non jumpgate games and add resilience to map importer

6f489cf

		if (firstSheet.layouthtml.indexOf("shaped_d20") > 0) d20plus.sheet = "shaped";
		if (firstSheet.layouthtml.indexOf("DnD5e_Character_Sheet") > 0) d20plus.sheet = "community";

Conversation

Zalatos commented Apr 1, 2025

Uh oh!

TheGiddyLimit 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!

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!

Zalatos Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

TheGiddyLimit commented Apr 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Zalatos commented Apr 7, 2025

Uh oh!

Zalatos commented Apr 7, 2025

Uh oh!

Zalatos commented Apr 7, 2025

Uh oh!

Zalatos commented Apr 8, 2025

Uh oh!

TheGiddyLimit commented Apr 14, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Zalatos Apr 7, 2025 •

edited

Loading

TheGiddyLimit commented Apr 6, 2025 •

edited

Loading