Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Order Flipping (and sorted some tab issues) #82

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
27 changes: 14 additions & 13 deletions src/Combatants.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,29 +11,30 @@ class Combatants extends Component {
}
render() {
const maxRows = this.props.config.maxCombatants
const dataArray = Object.keys(this.props.data)
const battler = dataArray.filter(player => (
const dataArray = this.props.isWebSocket ? Object.keys(this.props.data).reverse() : Object.keys(this.props.data)
const battlers = dataArray.filter(player => (
this.props.data[player].name.toLowerCase() !== 'limit break'
&& (!this.props.config.showJobless && this.props.data[player].Job && this.props.data[player].Job !== '') //doesn't have a job, filter it out.
&& (this.props.data[player].ENCDPS > 0 || this.props.data[player].ENCHPS > 0) //irrelevant npcs (i.e. estinien) like to show up for whatever reason
)).slice(-maxRows)
&& (!this.props.config.showJobless && this.props.data[player].Job && this.props.data[player].Job !== '') //doesn't have a job, filter it out.
&& (this.props.data[player].ENCDPS > 0 || this.props.data[player].ENCHPS > 0) //irrelevant npcs (i.e. estinien) like to show up for whatever reason
)).slice(0, maxRows)
let rows = []
let combatant
let isSelf
let currentRow = 1

for (const ref in battler) {
combatant = this.props.data[battler[ref]]
for (const battler of battlers) {
const combatant = this.props.data[battler]

// console.log(combatant)

// We'll change the global 'YOU' name in case it's, well, you
// In case you changedw your name in ACT and in the overlay config
isSelf =
// In case you changed your name in ACT and in the overlay config
const isSelf =
combatant.name.toUpperCase() === 'YOU' ||
this.props.config.characterName === combatant.name

// We need to reasign it here since it will call a reference
const rank = parseInt(ref, 10) + 1
const rank = currentRow

currentRow = currentRow + 1

// don't need to render this component if this is a limit break
// if (!combatant.name.toLowerCase() === 'limit break')
Expand All @@ -44,7 +45,7 @@ class Combatants extends Component {
data={combatant}
config={this.props.config}
isSelf={isSelf}
key={battler[ref]}
key={battler}
/>
)
}
Expand Down
27 changes: 16 additions & 11 deletions src/Overlay.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,14 @@ class OverlayRaw extends React.Component {
if (Object.getOwnPropertyNames(this.props.Combatant).length === 0)
return false

let maxRows = this.props.config.maxCombatants
let dataArray = Object.keys(this.props.Combatant)
let battler = dataArray.slice(0, maxRows)
let combatant
const maxRows = this.props.config.maxCombatants //not sure why let was being used here
Copy link
Owner

Choose a reason for hiding this comment

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

Probably because I changed it after, I was learning es6 at this time and didn't have a linter on - good catch though :)

const dataArray = this.props.isWebSocket ? Object.keys(this.props.Combatant).reverse() : Object.keys(this.props.Combatant) //const is fine, actws flips order for some reason I don't know why, this is my 'fix'
let discordData = []
let currentRow = 0

for (const ref in battler) {
combatant = this.props.Combatant[battler[ref]]

// Send to Discord the right name in Settings
if (combatant.name.toUpperCase() === 'YOU')
combatant.name = this.props.config.characterName
//NOTE: instead of wasting time slicing the array up we just break the loop when we need to, it avoids the problem of filtering out the lb only to want to handle it later
for (const battler of dataArray) {
const combatant = this.props.Combatant[battler] //scope means we can just use const here
Copy link
Owner

Choose a reason for hiding this comment

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

I thank you for explaining your steps, I know it may look weird in some places but I never refactored since v1 😉

Could you then remove the comments? They won't make sense after the PR is accepted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I will remove the comments I'm busy with savage raids and life at the moment but I'll get round to it as soon as possible


// Send limit break data separated
if (combatant.name.toLowerCase() === 'limit break') {
Expand All @@ -40,8 +36,16 @@ class OverlayRaw extends React.Component {
10
)
)
break
continue //break will just break the loop entirely.. if someone is deaing less damage than LB (yikes) they wouldn't end up included
Copy link
Owner

Choose a reason for hiding this comment

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

Here too if you can

}

if(currentRow >= maxRows) continue //sadly because of the above statement we have to loop through the full data set if we want to be sure

currentRow = currentRow + 1

// Send to Discord the right name in Settings
if (combatant.name.toUpperCase() === 'YOU')
combatant.name = this.props.config.characterName

discordData.push({
job: combatant.Job,
Expand Down Expand Up @@ -74,6 +78,7 @@ class OverlayRaw extends React.Component {
style={{ zoom: props.config.zoom }}
>
<Combatants
isWebSocket={props.isWebSocket}
data={props.Combatant}
encounterDamage={props.Encounter.damage}
config={props.config}
Expand Down
3 changes: 2 additions & 1 deletion src/actwebsocket.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ let querieSet = undefined
const getHost = () => /HOST_PORT=(wss?:\/\/.+)/.exec(window.location.search)

export default function initActWebSocket() {
if (!getHost()) return
if (!getHost()) return false
var webs
const url = new URLSearchParams(window.location.search)
const wsUri = `${url.get('HOST_PORT')}MiniParse` || undefined
Expand All @@ -28,6 +28,7 @@ export default function initActWebSocket() {
)
}
}
return true
}

Copy link
Owner

Choose a reason for hiding this comment

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

Why are we returning boolean?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need to identify if ACTWebSocket is in use to flip the order in a couple places, we don't want to perform regex (easiest way to check that is old chromium compatible) everytime a new render is requested and gone throguh the DPS so we pass it through here as a prop to components that need the information

Copy link
Owner

Choose a reason for hiding this comment

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

I think I didn't understand 😢

I'm asking why did you change this specific function to return a boolean instead of void (no value or undefined). I understood you need to check if ACTWebSocket is in use but from what I could see it's not from the value of this function, right? Or is it?

class ActWebsocketInterface {
Expand Down
4 changes: 2 additions & 2 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import initActWebSocket from './actwebsocket'

require(`./images/handle.png`)

initActWebSocket()
let isWS = initActWebSocket()

// Raven.config(sentryUrl).install()

Expand Down Expand Up @@ -67,7 +67,7 @@ function onOverlayDataUpdate(e) {
// }
const detail = (e.detail.msg ? e.detail.msg : e.detail)

ReactDOM.render(<Root {...detail} />, document.getElementById('root'))
ReactDOM.render(<Root {...detail} isWebSocket={isWS} />, document.getElementById('root'))
}
// This will run when there's no data
ReactDOM.render(<Inactive />, document.getElementById('root'))
Expand Down
8 changes: 4 additions & 4 deletions src/locale.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const locale = {
toggleOption9: 'Discord',
toggleOption10: 'Language',
toggleOption11: 'Max Hit',
toggleOption12: 'Show "jobless" Combatants',
toggleOption12: 'Show "jobless" Combatants',
maxCombatantsTitle: 'Max Combatants',
zoomTitle: 'Zoom Scale',
zoomOption1: '80%',
Expand Down Expand Up @@ -78,7 +78,7 @@ const locale = {
toggleOption9: 'Discord',
toggleOption10: 'Língua',
toggleOption11: 'Max Hit',
toggleOption12: 'Show "jobless" Combatants',
toggleOption12: 'Show "jobless" Combatants',
maxCombatantsTitle: 'Máx. Personagens',
zoomTitle: 'Escala de Zoom',
zoomOption1: '80%',
Expand Down Expand Up @@ -133,7 +133,7 @@ const locale = {
toggleOption9: 'Discord',
toggleOption10: '模版语言',
toggleOption11: '最强伤害',
toggleOption12: 'Show "jobless" Combatants',
toggleOption12: 'Show "jobless" Combatants',
maxCombatantsTitle: '最强战员',
zoomTitle: '缩放尺寸',
zoomOption1: '80%',
Expand Down Expand Up @@ -188,7 +188,7 @@ const locale = {
toggleOption9: 'Discord',
toggleOption10: '模版語言',
toggleOption11: '最強傷害',
toggleOption12: 'Show "jobless" Combatants',
toggleOption12: 'Show "jobless" Combatants',
maxCombatantsTitle: '最強戰員',
zoomTitle: '縮放尺寸',
zoomOption1: '80%',
Expand Down