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.

Sign up for GitHub

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

Jump to bottom

Add user interface option to Notify by Pokemon Level #2399

Merged
merged 7 commits into from
Jan 27, 2018
Merged

Add user interface option to Notify by Pokemon Level #2399

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ab964b3
Add user interface option to Notify By Pokemon Level
tomballgithub Dec 16, 2017
5d44772
Fix typo
tomballgithub Jan 19, 2018
62d9912
Fix rebase issue
tomballgithub Jan 23, 2018
317f365
Make changes based on review input
tomballgithub Jan 23, 2018
ac92978
Cleanup
tomballgithub Jan 23, 2018
0f3cb05
Remove errant const
tomballgithub Jan 23, 2018
5e7a62f
Fix conflict in logic.
sebastienvercammen Jan 26, 2018
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
4 changes: 4 additions & 0 deletions static/js/map.common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -866,6 +866,10 @@ var StoreOptions = {
default: '',
type: StoreTypes.Number
},
'remember_text_level_notify': {
Copy link
Contributor

Choose a reason for hiding this comment

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

js object literal syntax error, npm run build will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, fixed. Was introduced during rebase.

default: 0,
type: StoreTypes.Number
},
'excludedRarity': {
default: 0, // 0: none, 1: <=Common, 2: <=Uncommon, 3: <=Rare, 4: <=Very Rare, 5: <=Ultra Rare
type: StoreTypes.Number
Expand Down
29 changes: 27 additions & 2 deletions static/js/map.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ var $selectExclude
var $selectPokemonNotify
var $selectRarityNotify
var $textPerfectionNotify
var $textLevelNotify
var $selectStyle
var $selectIconSize
var $switchOpenGymsOnly
Expand Down Expand Up @@ -37,6 +38,7 @@ var excludedRarity
var notifiedPokemon = []
var notifiedRarity = []
var notifiedMinPerfection = null
var notifiedMinLevel = null

var buffer = []
var reincludedPokemon = []
Expand Down Expand Up @@ -1046,13 +1048,23 @@ function playPokemonSound(pokemonID, cryFileTypes) {

function isNotifyPerfectionPoke(poke) {
var hasHighIV = false
var hasHighLevel = false
var hasHighAttributes = false

if (poke['individual_attack'] != null) {
if (poke['individual_attack'] != null && poke['cp_multiplier'] !== null) {
const perfection = getIv(poke['individual_attack'], poke['individual_defense'], poke['individual_stamina'])
const level = getPokemonLevel(poke['cp_multiplier'])

hasHighIV = notifiedMinPerfection > 0 && perfection >= notifiedMinPerfection
hasHighLevel = notifiedMinLevel > 0 && level >= notifiedMinLevel

const shouldNotifyForIV = (hasHighIV && notifiedMinLevel <= 0)
const shouldNotifyForLevel = (hasHighLevel && (hasHighIV || notifiedMinPerfection <= 0))

hasHighAttributes = shouldNotifyForIV || shouldNotifyForLevel
}

return hasHighIV
return hasHighAttributes
Copy link
Member

Choose a reason for hiding this comment

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

The current code will cause a problem when "Notify by IV" is enabled and "Notify by level" is disabled, and the Pokémon has IV data but no cp_multiplier (technically not possible since they're both inserted during scan, but logically they're unrelated so they should be separated).

The problem lies in your condition:

var hasHighAttributes = false

if (poke['individual_attack'] != null && poke['cp_multiplier'] !== null) {
    ...
}

return hasHighAttributes

}

function isNotifyPoke(poke) {
Expand Down Expand Up @@ -2687,6 +2699,7 @@ $(function () {
$selectPokemonNotify = $('#notify-pokemon')
$selectRarityNotify = $('#notify-rarity')
$textPerfectionNotify = $('#notify-perfection')
$textLevelNotify = $('#notify-level')
var numberOfPokemon = 493

// Load pokemon names and populate lists
Expand Down Expand Up @@ -2768,13 +2781,25 @@ $(function () {
$textPerfectionNotify.val(notifiedMinPerfection)
Store.set('remember_text_perfection_notify', notifiedMinPerfection)
})
$textLevelNotify.on('change', function (e) {
notifiedMinLevel = parseInt($textLevelNotify.val(), 10)
if (isNaN(notifiedMinLevel) || notifiedMinLevel <= 0) {
notifiedMinLevel = ''
Copy link
Member

Choose a reason for hiding this comment

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

The default should be 0 rather than an empty string and the <= 0 should be changed with < 0. 0 can be a valid option, to disable it. Make sure to keep the behavior consistent with "Notify by IV".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree but note that I copied the exact '' assignment also used for remember_text_perfection_notify

}
if (notifiedMinLevel > 35) {
notifiedMinLevel = 35
}
$textLevelNotify.val(notifiedMinLevel)
Store.set('remember_text_level_notify', notifiedMinLevel)
})

// recall saved lists
$selectExclude.val(Store.get('remember_select_exclude')).trigger('change')
$selectExcludeRarity.val(Store.get('excludedRarity')).trigger('change')
$selectPokemonNotify.val(Store.get('remember_select_notify')).trigger('change')
$selectRarityNotify.val(Store.get('remember_select_rarity_notify')).trigger('change')
$textPerfectionNotify.val(Store.get('remember_text_perfection_notify')).trigger('change')
$textLevelNotify.val(Store.get('remember_text_level_notify')).trigger('change')

if (isTouchDevice() && isMobileDevice()) {
$('.select2-search input').prop('readonly', true)
Expand Down
6 changes: 6 additions & 0 deletions templates/map.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ <h3>Notify of Perfection</h3>
<input id="notify-perfection" type="text" name="notify-perfection" placeholder="Minimum perfection %"/>
</label>
</div>
</div>
<div class="form-control">
<label for="notify-level">
<h3>Notify of Level</h3>
<input id="notify-level" type="text" name="notify-level" placeholder="Minimum level"/>
</label>
</div>
{% endif %}
<div class="form-control switch-container">
Expand Down