Rollv2 by mgerni · Pull Request #82 · OSRS-Taskman/Taskapp_Public

mgerni · 2026-02-22T00:42:11Z

No description provided.

mgerni · 2026-02-22T00:43:34Z

Copilot · 2026-02-22T00:43:41Z

@mgerni I've opened a new pull request, #83, to work on those changes. Once the pull request is ready, I'll request review from you.

Copilot

Pull request overview

This pull request introduces an animated task rolling feature ("Rollv2") that provides a slot-machine-style animation when generating tasks for both official and unofficial game modes. The changes replace the previous frame-by-frame animation with a more polished rolling animation that displays available tasks before landing on the generated task.

Changes:

Replaced frame-cycling animation with a slot-machine-style rolling animation for task generation
Added new backend endpoint to fetch available tasks for the rolling animation pool
Simplified the empty task state UI by removing multiple decorative images and messages
Removed extensive commented-out HTML code (135+ lines of duplicated legacy code)

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
templates/dashboard_official.html	Simplified empty task state from 7 images/messages to 1 each; removed 135+ lines of commented legacy code
taskapp.py	Added `/available_roll_tasks/` endpoint to fetch uncompleted tasks for the rolling animation pool
task_database.py	Added `get_available_roll_tasks()` function to retrieve uncompleted tasks by tier with optional tier parameter
static/styles.css	Added CSS classes for the new rolling modal UI including progress bar, reel, and responsive layout
static/js/task.js	Completely rewrote task generation logic with new animation system, modal management, and special user handling

Comments suppressed due to low confidence (2)

static/js/task.js:89

The progress bar lacks proper ARIA attributes for accessibility. Screen readers won't be able to announce progress updates to users with visual impairments. Consider adding role="progressbar", aria-valuenow, aria-valuemin, and aria-valuemax attributes to the progress bar div, and update aria-valuenow when the progress changes.

      <div class="task-action-progress-wrap task-roll-progress-wrap">
        <div class="task-action-progress-bar task-roll-progress-bar">
          <div id="taskRollProgressFill" class="task-roll-progress-fill"></div>
        </div>

static/js/task.js:59

The function uses Math.pow which is less performant than the exponentiation operator (**). Consider replacing Math.pow(progress, 2) with progress ** 2 for better performance and readability, following modern JavaScript conventions.

    const delay = 30 + Math.floor(Math.pow(progress, 2) * 150);

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-22T00:57:10Z

taskapp.py

+@app.route('/available_roll_tasks/', methods=['POST'])
+@login_required
+def available_roll_tasks():
+    tier = request.form.get('tier')
+    _, tasks = get_available_roll_tasks(session['username'], tier)
+    return {
+        'tasks': [
+            {
+                'name': task.name,
+                'image': task.image_link,
+                'tip': task.tip,
+                'link': task.wiki_link,
+            }
+            for task in tasks
+        ]
+    }


The new endpoint lacks error handling. If get_available_roll_tasks raises an exception (e.g., user not found), the endpoint will return a 500 error with no meaningful message. Consider adding try-except blocks similar to other endpoints in the codebase, or at minimum add validation to ensure the function returns valid data before constructing the response.

Copilot · 2026-02-22T00:57:10Z

static/js/task.js

+  modal.addEventListener('cancel', (event) => {
+    if (confirmButton.disabled) {
+      event.preventDefault();
+    }
+  }, { once: true });


The 'cancel' event listener is added with { once: true } each time showTaskRollModal is called. If the modal is reused multiple times without being recreated (which ensureTaskRollModal does), subsequent calls won't register new event listeners since the previous ones are removed after firing once. This could lead to inconsistent behavior. Consider either always recreating the modal, or managing event listeners more carefully to ensure they're properly attached for each modal display.

Suggested change

modal.addEventListener('cancel', (event) => {

if (confirmButton.disabled) {

event.preventDefault();

}

}, { once: true });

// Ensure we don't accumulate multiple cancel listeners across calls.

if (modal._taskRollCancelHandler) {

modal.removeEventListener('cancel', modal._taskRollCancelHandler);

}

const cancelHandler = (event) => {

if (confirmButton.disabled) {

event.preventDefault();

}

};

modal._taskRollCancelHandler = cancelHandler;

modal.addEventListener('cancel', cancelHandler);

Copilot · 2026-02-22T00:57:11Z

static/styles.css

+    margin-bottom: 12px;
+}
+
+.task-roll-progress-bar {


The .task-roll-progress-bar is missing critical CSS properties that are present in the similar .task-action-progress-bar class (lines 623-629). Specifically, it's missing: position: relative (required for the absolutely-positioned fill to work correctly), border: 1px solid #606060, overflow: hidden, and background: #3f3930. Without these properties, the progress bar will not render correctly. Copy the missing properties from .task-action-progress-bar to maintain visual consistency.

Suggested change

.task-roll-progress-bar {

.task-roll-progress-bar {

position: relative;

border: 1px solid #606060;

overflow: hidden;

background: #3f3930;

Copilot · 2026-02-22T00:57:11Z

static/js/task.js

+function isSpecialInstantRollUser() {
+  const username = getCurrentUsername().toLowerCase();
+  return username === 'shadukat' || username === 'gerni shadu';


This function hardcodes specific usernames ('shadukat' and 'gerni shadu') to provide special behavior. Hardcoded usernames in business logic create maintainability concerns and make the code less flexible. Consider moving these special user identifiers to a configuration file or database, or implementing a user role/flag system that can be managed through the admin interface rather than requiring code changes.

Suggested change

function isSpecialInstantRollUser() {

const username = getCurrentUsername().toLowerCase();

return username === 'shadukat' || username === 'gerni shadu';

function getSpecialInstantRollUsers() {

if (typeof window !== 'undefined' &&

Array.isArray(window.SPECIAL_INSTANT_ROLL_USERS) &&

window.SPECIAL_INSTANT_ROLL_USERS.length > 0) {

return window.SPECIAL_INSTANT_ROLL_USERS

.map((name) => (typeof name === 'string' ? name.toLowerCase() : ''))

.filter(Boolean);

}

// Default special users for instant roll behavior; can be overridden

// by defining window.SPECIAL_INSTANT_ROLL_USERS elsewhere.

return ['shadukat', 'gerni shadu'];

}

function isSpecialInstantRollUser() {

const username = getCurrentUsername().toLowerCase();

if (!username) {

return false;

}

const specialUsers = getSpecialInstantRollUsers();

return specialUsers.indexOf(username) !== -1;

Copilot · 2026-02-22T00:57:11Z

static/js/task.js

+      sequence.push(chunk[i]);
+    }
+  }
+  sequence.push(finalTask);


When the final task is null or undefined (if generation fails), this will create a sequence ending with null. The subsequent code in showTaskRollModal may fail when trying to access properties on the finalTask. Consider adding validation to handle cases where finalTask might be null/undefined, or ensure the calling code validates this before building the sequence.

Suggested change

sequence.push(finalTask);

if (finalTask) {

sequence.push(finalTask);

}

Copilot · 2026-02-22T00:57:11Z

taskapp.py

+@app.route('/available_roll_tasks/', methods=['POST'])
+@login_required
+def available_roll_tasks():
+    tier = request.form.get('tier')
+    _, tasks = get_available_roll_tasks(session['username'], tier)
+    return {
+        'tasks': [
+            {
+                'name': task.name,
+                'image': task.image_link,
+                'tip': task.tip,
+                'link': task.wiki_link,
+            }
+            for task in tasks
+        ]
+    }


The endpoint doesn't validate the tier parameter before passing it to get_available_roll_tasks. If an invalid tier value is provided (e.g., through a malicious request or bug in the frontend), it will cause an unhandled KeyError when tasklists.list_for_tier tries to look up the tier. Consider adding input validation to ensure tier is one of the expected values ('easy', 'medium', 'hard', 'elite', 'master') or None, and return an appropriate error response for invalid values.

Rollv2: Diagnose and document merge conflict resolution path

mgerni added 4 commits February 21, 2026 19:23

updated rolling animation

a4e2111

fix merge conflict for roll animation

48b4b9f

updated titles

85646b5

fix all the things

c23fa9c

mgerni requested a review from Copilot February 22, 2026 00:42

Copilot started reviewing on behalf of mgerni February 22, 2026 00:42 View session

Initial plan

3ed1728

Copilot AI mentioned this pull request Feb 22, 2026

Rollv2: Diagnose and document merge conflict resolution path #83

Merged

Copilot AI reviewed Feb 22, 2026

View reviewed changes

Merge pull request #83 from OSRS-Taskman/copilot/sub-pr-82

c045149

Rollv2: Diagnose and document merge conflict resolution path

mgerni closed this Feb 22, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rollv2#82

Rollv2#82
mgerni wants to merge 6 commits intomasterfrom
rollv2

mgerni commented Feb 22, 2026

Uh oh!

mgerni commented Feb 22, 2026

Uh oh!

Copilot AI commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Copilot AI Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-.task-roll-progress-bar {
+.task-roll-progress-bar {
+    position: relative;
+    border: 1px solid #606060;
+    overflow: hidden;
+    background: #3f3930;

-function isSpecialInstantRollUser() {
-  const username = getCurrentUsername().toLowerCase();
-  return username === 'shadukat' || username === 'gerni shadu';
+function getSpecialInstantRollUsers() {
+  if (typeof window !== 'undefined' &&
+      Array.isArray(window.SPECIAL_INSTANT_ROLL_USERS) &&
+      window.SPECIAL_INSTANT_ROLL_USERS.length > 0) {
+    return window.SPECIAL_INSTANT_ROLL_USERS
+      .map((name) => (typeof name === 'string' ? name.toLowerCase() : ''))
+      .filter(Boolean);
+  }
+  // Default special users for instant roll behavior; can be overridden
+  // by defining window.SPECIAL_INSTANT_ROLL_USERS elsewhere.
+  return ['shadukat', 'gerni shadu'];
+}
+function isSpecialInstantRollUser() {
+  const username = getCurrentUsername().toLowerCase();
+  if (!username) {
+    return false;
+  }
+  const specialUsers = getSpecialInstantRollUsers();
+  return specialUsers.indexOf(username) !== -1;

Conversation

mgerni commented Feb 22, 2026

Uh oh!

mgerni commented Feb 22, 2026

Uh oh!

Copilot AI commented Feb 22, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants