Skip to content

Commit

Permalink
Fix ad initialization race condition, which was causing ads to someti…
Browse files Browse the repository at this point in the history
…mes not get displayed, especially in 2nd tabs
  • Loading branch information
dumbmatter committed Feb 27, 2023
1 parent fa50040 commit 98c949d
Show file tree
Hide file tree
Showing 6 changed files with 35 additions and 34 deletions.
6 changes: 0 additions & 6 deletions TODO
Original file line number Diff line number Diff line change
@@ -1,9 +1,3 @@
female
- update face editor
- blog and test links
- add clickable face to blog post
- make onFaceJS an array or use events or something

make cap space visible above the fold on trade page on mobile https://mail.google.com/mail/u/0/#inbox/FMfcgzGrcXqKVMHkvwNxkzXsrsbWwBKh

jordan not in HoF when starting in 2003 during draft https://discord.com/channels/290013534023057409/290015591216054273/1073981357614706821
Expand Down
35 changes: 19 additions & 16 deletions src/ui/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,29 @@ import type {
UpdateEvents,
GameAttributesLeague,
} from "../../common/types";
import { AD_DIVS, GRACE_PERIOD } from "../../common";
import { AD_DIVS } from "../../common";
import { updateSkyscraperDisplay } from "../components/Skyscraper";

// Read from goldUntil rather than local because this is called before local is updated
const initAds = (goldUntil: number | undefined) => {
let hideAds = false; // No ads for Gold members

const currentTimestamp = Math.floor(Date.now() / 1000) - GRACE_PERIOD;

if (goldUntil === undefined || currentTimestamp < goldUntil) {
hideAds = true;
let accountChecked = false;
let uiRendered = false;
const initAds = (type: "accountChecked" | "uiRendered") => {
// Prevent race condition by assuring we run this only after the account has been checked and the UI has been rendered, otherwise (especially when opening a 2nd tab) this was sometimes running before the UI was rendered, which resulted in no ads being displayed
if (accountChecked && uiRendered) {
// Must have already ran somehow?
return;
}
if (type === "accountChecked") {
accountChecked = true;
} else if (type === "uiRendered") {
uiRendered = true;
}
if (!accountChecked || !uiRendered) {
return;
}

const gold = local.getState().gold;

if (!hideAds) {
if (!gold) {
// _disabled names are to hide from Blockthrough, so it doesn't leak through for Gold subscribers. Run this regardless of window.freestar, so Blockthrough can still work for some users.
const divsAll = [
AD_DIVS.mobile,
Expand Down Expand Up @@ -165,12 +174,6 @@ const initGold = () => {
div.id = `${id}_disabled`;
}
}
console.log(
"initGold end",
"display",
document.getElementById("basketball-gm_mobile_leaderboard")?.style
.display,
);
});
};

Expand Down
6 changes: 6 additions & 0 deletions src/ui/components/Controller.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import Skyscraper from "./Skyscraper";
import TitleBar from "./TitleBar";
import { useViewData } from "../util/viewManager";
import { isSport } from "../../common";
import api from "../api";

const loadFramerMotionFeatures = () =>
import("../util/framerMotionFeatures").then(res => res.default);
Expand Down Expand Up @@ -66,6 +67,11 @@ const Controller = () => {
}
}, [popup]);

useEffect(() => {
// Try to show ads on initial render
api.initAds("uiRendered");
}, []);

const {
Component,
data,
Expand Down
4 changes: 2 additions & 2 deletions src/ui/views/LoginOrRegister/Login.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { FormEvent, useRef, useState } from "react";
import { ACCOUNT_API_URL, fetchWrapper } from "../../../common";
import { ACCOUNT_API_URL, fetchWrapper, GRACE_PERIOD } from "../../../common";
import { ActionButton } from "../../components";
import {
analyticsEvent,
Expand Down Expand Up @@ -33,7 +33,7 @@ const Login = ({ ajaxErrorMsg }: { ajaxErrorMsg: string }) => {
});

if (data.success) {
const currentTimestamp = Math.floor(Date.now() / 1000);
const currentTimestamp = Math.floor(Date.now() / 1000) - GRACE_PERIOD;
const gold = currentTimestamp <= data.gold_until;
localActions.update({
gold,
Expand Down
14 changes: 6 additions & 8 deletions src/worker/api/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
gameAttributesArrayToObject,
DEFAULT_JERSEY,
POSITIONS,
GRACE_PERIOD,
} from "../../common";
import actions from "./actions";
import leagueFileUpload, {
Expand Down Expand Up @@ -2147,27 +2148,24 @@ const init = async (inputEnv: Env, conditions: Conditions) => {
(async () => {
// Account check needs to complete before initAds, though
await checkAccount(conditions);
await toUI("initAds", [local.goldUntil], conditions);
await toUI("initAds", ["accountChecked"], conditions);

// This might make another HTTP request, and is less urgent than ads
await checkChanges(conditions);
})();
} else {
// No need to run checkAccount and make another HTTP request
const currentTimestamp = Math.floor(Date.now() / 1000);
const currentTimestamp = Math.floor(Date.now() / 1000) - GRACE_PERIOD;
await toUI("updateLocal", [
{
gold: local.goldUntil < Infinity && currentTimestamp <= local.goldUntil,
username: local.username,
},
]);

// Even if it's not the first host tab, show ads (still async). Why
// setTimeout? Cause horrible race condition with actually rendering the
// ad divs. Need to move them more fully into React to solve this.
setTimeout(() => {
toUI("initAds", [local.goldUntil], conditions);
}, 0);
(async () => {
await toUI("initAds", ["accountChecked"], conditions);
})();
}

// Send options to all new tabs
Expand Down
4 changes: 2 additions & 2 deletions src/worker/util/checkAccount.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ACCOUNT_API_URL, fetchWrapper } from "../../common";
import { ACCOUNT_API_URL, fetchWrapper, GRACE_PERIOD } from "../../common";
import { idb } from "../db";
import achievement from "./achievement";
import local from "./local";
Expand Down Expand Up @@ -26,7 +26,7 @@ const checkAccount = async (
local.goldUntil = data.gold_until;
local.mailingList = !!data.mailing_list;
local.username = data.username === "" ? undefined : data.username;
const currentTimestamp = Math.floor(Date.now() / 1000);
const currentTimestamp = Math.floor(Date.now() / 1000) - GRACE_PERIOD;
await toUI("updateLocal", [
{
gold: currentTimestamp <= data.gold_until,
Expand Down

0 comments on commit 98c949d

Please sign in to comment.