Skip to content
Draft
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
182 changes: 91 additions & 91 deletions dist/betteR20-5etools.user.js

Large diffs are not rendered by default.

178 changes: 89 additions & 89 deletions dist/betteR20-core.user.js

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions js/5etools-main.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ const betteR205etoolsMain = function () {
{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`},
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

yeah that endpoint responded with 404

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

image

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

// {name: "adventures index", url: `${DATA_URL}adventures.json`}, // error retrieving from 5e.tools
{name: "base items", url: `${DATA_URL}items-base.json`},
{name: "item modifiers", url: `${DATA_URL}roll20-items.json`},
];
Expand Down Expand Up @@ -906,7 +906,8 @@ const betteR205etoolsMain = function () {
d20plus.setSheet = function () {
d20plus.ut.log("Switched Character Sheet Template");
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) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

sheets.length < 1 -> !sheets.length

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

const $body = $(`body`);
$body.addClass("ve-nosheet__body");
const $btnClose = $(`<button class="btn btn-danger ve-nosheet__btn-close">X</button>`)
Expand All @@ -929,8 +930,9 @@ const betteR205etoolsMain = function () {
}));
throw new Error("No character sheet selected!");
}
if (d20.journal.customSheets.layouthtml.indexOf("shaped_d20") > 0) d20plus.sheet = "shaped";
if (d20.journal.customSheets.layouthtml.indexOf("DnD5e_Character_Sheet") > 0) d20plus.sheet = "community";
const firstSheet = d20.journal.customSheets ?? sheets.first();
Comment thread
TheGiddyLimit marked this conversation as resolved.
if (firstSheet.layouthtml.indexOf("shaped_d20") > 0) d20plus.sheet = "shaped";
if (firstSheet.layouthtml.indexOf("DnD5e_Character_Sheet") > 0) d20plus.sheet = "community";
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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")

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

sweet i like it

d20plus.ut.log(`Switched Character Sheet Template to ${d20plus.sheet}`);
};

Expand Down
40 changes: 21 additions & 19 deletions js/5etools-template.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,27 +93,29 @@ const d20plusTemplate = function () {
}

d20plus.template5e._populateAdventuresDropdown = function () {
const defaultAdvUrl = d20plus.formSrcUrl(ADVENTURE_DATA_DIR, "adventure-lmop.json");
const $iptUrl = $("#import-adventures-url");
$iptUrl.val(defaultAdvUrl);
$iptUrl.data("id", "lmop");
const $sel = $("#button-adventures-select");
adventureMetadata.adventure.forEach(a => {
if (adventureMetadata.adventure){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

const defaultAdvUrl = d20plus.formSrcUrl(ADVENTURE_DATA_DIR, "adventure-lmop.json");
const $iptUrl = $("#import-adventures-url");
$iptUrl.val(defaultAdvUrl);
$iptUrl.data("id", "lmop");
const $sel = $("#button-adventures-select");
adventureMetadata.adventure.forEach(a => {
$sel.append($("<option>", {
value: d20plus.formSrcUrl(ADVENTURE_DATA_DIR, `adventure-${a.id.toLowerCase()}.json|${a.id}`),
text: a.name,
}));
});
$sel.append($("<option>", {
value: d20plus.formSrcUrl(ADVENTURE_DATA_DIR, `adventure-${a.id.toLowerCase()}.json|${a.id}`),
text: a.name,
value: "",
text: "Custom",
}));
});
$sel.append($("<option>", {
value: "",
text: "Custom",
}));
$sel.val(defaultAdvUrl);
$sel.change(() => {
const [url, id] = $sel.val().split("|");
$($iptUrl).val(url);
$iptUrl.data("id", id);
});
$sel.val(defaultAdvUrl);
$sel.change(() => {
const [url, id] = $sel.val().split("|");
$($iptUrl).val(url);
$iptUrl.data("id", id);
});
}
}

d20plus.template5e.addCustomHTML = function () {
Expand Down
5 changes: 3 additions & 2 deletions js/base-chat.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,15 @@ function baseChat () {

function availableLanguages (charId) {
const char = d20.Campaign.characters.get(charId);
const langId = d20.journal.customSheets.availableAttributes.repeating_proficiencies_prof_type;
const firstCharSheet = d20.journal.customSheets ?? d20.journal.characterSheetsManager.getAllSheets().first();
const langId = firstCharSheet.availableAttributes.repeating_proficiencies_prof_type;
if (!char) return [];
if (!char.attribs.length) {
const fetched = d20plus.ut.fetchCharAttribs(char);
fetched.then(d20plus.chat.refreshLanguages);
}
// roll20 OGL sheet stores languages differently compared to other traits
// by default, they don't have corresponging "proficiency type" attribute
// by default, they don't have corresponding "proficiency type" attribute
// however, if you create a trait and THEN change it to be language, it will have LOCALIZED "language" proficiency type
// so to find all languages, we must filter out other named traits, except for the traits named "language" or "(localized word for LANGUAGE)"
const traits = char.attribs.models
Expand Down
68 changes: 40 additions & 28 deletions js/base-engine.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ function d20plusEngine () {
$(`head`).append(`<style id="5etools-status-css"/>`);

d20plus.mod.overwriteStatusEffects();
if (!d20.engine.canvas.on) {
d20.engine.canvas.prototype.on = $.on ?? $("babylonCanvas").on;
d20.engine.canvas.prototype.off = $.off ?? $("babylonCanvas").off;
}

d20.engine.canvas.off("object:added");
d20.engine.canvas.on("object:added", d20plus.mod.overwriteStatusEffects);
Expand Down Expand Up @@ -528,33 +532,35 @@ function d20plusEngine () {

// needs to be called after `enhanceMeasureTool()`
d20plus.engine.enhanceMouseMove = () => {
// add missing vars
var i = d20.engine.canvas;

// Roll20 bug (present as of 2019-5-25) workaround
// when box-selecting + moving tokens, the "object:moving" event throws an exception
// try-catch-ignore this, because it's extremely annoying
const cachedFire = i.fire.bind(i);
i.fire = function (namespace, opts) {
if (namespace === "object:moving") {
try {
if (d20.engine.canvas){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

prefer inverting and avoiding the extra indent, i.e.

if (!d20.engine.canvas) return;

...

// add missing vars
var i = d20.engine.canvas;

// Roll20 bug (present as of 2019-5-25) workaround
// when box-selecting + moving tokens, the "object:moving" event throws an exception
// try-catch-ignore this, because it's extremely annoying
const cachedFire = i.fire.bind(i);
i.fire = function (namespace, opts) {
if (namespace === "object:moving") {
try {
cachedFire(namespace, opts);
} catch (e) {}
} else {
cachedFire(namespace, opts);
} catch (e) {}
} else {
cachedFire(namespace, opts);
}
};
}
};

const I = d20plus.overwrites.canvasHandlerMove
const I = d20plus.overwrites.canvasHandlerMove

if (FINAL_CANVAS_MOUSEMOVE_LIST.length) {
FINAL_CANVAS_MOUSEMOVE = (FINAL_CANVAS_MOUSEMOVE_LIST.find(it => it.on === d20.engine.final_canvas) || {}).listener;
}
if (FINAL_CANVAS_MOUSEMOVE_LIST.length) {
FINAL_CANVAS_MOUSEMOVE = (FINAL_CANVAS_MOUSEMOVE_LIST.find(it => it.on === d20.engine.final_canvas) || {}).listener;
}

if (FINAL_CANVAS_MOUSEMOVE) {
d20plus.ut.log("Enhancing mouse move");
d20.engine.final_canvas.removeEventListener("mousemove", FINAL_CANVAS_MOUSEMOVE);
d20.engine.final_canvas.addEventListener("mousemove", I);
if (FINAL_CANVAS_MOUSEMOVE) {
d20plus.ut.log("Enhancing mouse move");
d20.engine.final_canvas.removeEventListener("mousemove", FINAL_CANVAS_MOUSEMOVE);
d20.engine.final_canvas.addEventListener("mousemove", I);
}
}
};

Expand Down Expand Up @@ -589,6 +595,11 @@ function d20plusEngine () {
cacheRenderLoop();
};

if (!d20.engine.canvas.on) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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:

image

Copy link
Copy Markdown
Author

@Zalatos Zalatos Apr 7, 2025

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

d20.engine.canvas.prototype.on = $.on ?? $("babylonCanvas").on;
d20.engine.canvas.prototype.off = $.off ?? $("babylonCanvas").off;
}

// store data for the rendering function to access
d20.engine.canvas.on("mouse:move", (data, ...others) => {
// enable hover from GM layer -> token layer
Expand Down Expand Up @@ -748,11 +759,12 @@ function d20plusEngine () {
`);
}
}

d20.engine.canvas._renderAll = _.bind(d20plus.mod.renderAll, d20.engine.canvas);
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){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit: missing between ){

d20.engine.canvas._renderAll = _.bind(d20plus.mod.renderAll, d20.engine.canvas);
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);
}
};

d20plus.engine.removeLinkConfirmation = function () {
Expand Down
26 changes: 14 additions & 12 deletions js/base-menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,19 @@ function baseMenu () {

const getTokenWhisperPart = () => d20plus.cfg.getOrDefault("token", "massRollWhisperName") ? "/w gm Rolling for @{selected|token_name}...\n" : "";

Mousetrap.bind("b b", function () { // back on layer
const n = d20plus.engine.getSelectedToMove();
d20plus.engine.backwardOneLayer(n);
return false;
});

Mousetrap.bind("b f", function () { // forward one layer
const n = d20plus.engine.getSelectedToMove();
d20plus.engine.forwardOneLayer(n);
return false;
});
if (typeof(Mousetrap) !== "undefined"){
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nit typeof(Mousetrap) -> typeof Mousetrap

Mousetrap.bind("b b", function () { // back on layer
const n = d20plus.engine.getSelectedToMove();
d20plus.engine.backwardOneLayer(n);
return false;
});

Mousetrap.bind("b f", function () { // forward one layer
const n = d20plus.engine.getSelectedToMove();
d20plus.engine.forwardOneLayer(n);
return false;
});
}

/**
* @param token A token.
Expand Down Expand Up @@ -82,7 +84,7 @@ function baseMenu () {
var d = t.height()
, h = t.width()
, p = {};

// BEGIN MOD
// This block is pasted from newer version of roll20 Menu code, with appropriate changes to vars etc
const r20ping = (u,i)=>{
Expand Down
3 changes: 3 additions & 0 deletions js/base-remote-libre.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ function remoteLibre () {
return fetch("https://api.github.com/repos/DMsGuild201/Roll20_resources/contents/playlist")
.then(response => response.json())
.then(data => {
// check if blocked
if (data.block)
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

missing a space, but yes

if (conditionIsMet) doTheThing();

return Promise.reject(`Blocked: ${data.block.reason ?? "Unknown reason"}`);
const promises = data.filter(file => file.download_url.toLowerCase().endsWith(".json"))
.map(file => d20plus.remoteLibre.downloadPlaylist(file.download_url));
return Promise.all(promises).then(res => d20plus.remoteLibre.processRemotePlaylists(res));
Expand Down
4 changes: 2 additions & 2 deletions js/base-weather.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,8 @@ function baseWeather () {
cachedSetCanvasSize(e, n);
};

cv.width = cvBuf.width = d20.engine.canvas.width;
cv.height = cvBuf.height = d20.engine.canvas.height;
cv.width = cvBuf.width = d20.engine.canvasWidth;
cv.height = cvBuf.height = d20.engine.canvasHeight;

const ctx = cv.getContext("2d");

Expand Down
13 changes: 6 additions & 7 deletions js/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ const D20plus = function (version) {
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);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

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

d20plus.Init();
} else {
setTimeout(waitForD20, 50);
Expand All @@ -48,14 +49,12 @@ const D20plus = function (version) {

window.d20plus = d20plus;
d20plus.ut.log("Injection successful...");
} else if (timeWaitedForEnhancementSuiteMs > 4 * 5000) {
alert("betteR20 may require the VTTES (R20ES) extension to be installed!\nPlease install it from https://ssstormy.github.io/roll20-enhancement-suite/\nClicking ok will take you there.");
window.open("https://ssstormy.github.io/roll20-enhancement-suite/", "_blank");
} else {
if (timeWaitedForEnhancementSuiteMs > 4 * 5000) {
alert("betteR20 may require the VTTES (R20ES) extension to be installed!\nPlease install it from https://ssstormy.github.io/roll20-enhancement-suite/\nClicking ok will take you there.");
window.open("https://ssstormy.github.io/roll20-enhancement-suite/", "_blank");
} else {
timeWaitedForEnhancementSuiteMs += 100;
setTimeout(waitForEnhancementSuite, 100);
}
timeWaitedForEnhancementSuiteMs += 100;
setTimeout(waitForEnhancementSuite, 100);
}
})();
}
Expand Down