Feature: Warps need to be connected to the same electricity network

[bbassie]

This pull request contains a refactor of the warp list to enable to make it easier to implement this feature.

[bbassie]

Creating a warp will now error if you are standing too close to water, this should also fix being able to create land with warps.

[Cooldude2606]

• The new warp looks nice, makes me wonder if the center colour could be based on the icon selected, possibility through using a table lookup.
• The left padding on the text feilds needs to be set back to 0, the text looks to cramped being pushed against the edge.
• Warp proximity bypass is not working correctly, any role which has the bypass is unable to warp at all, the expected behaviour is that anyone with the bypass is able to goto any warp without the requirement of being on a warp. I suspect the issue is because the permission is not checked for during the new connected network check.
• Longer names break the scroll bar, although I think this might be a factorio issue.

[bbassie]

• The new warp looks nice, makes me wonder if the center colour could be based on the icon selected, possibility through using a table lookup.

Maybe for the future ;)

• The left padding on the text fields needs to be set back to 0, the text looks to cramped being pushed against the edge.

Added a bit more padding

• Warp proximity bypass is not working correctly, any role which has the bypass is unable to warp at all, the expected behavior is that anyone with the bypass is able to goto any warp without the requirement of being on a warp. I suspect the issue is because the permission is not checked for during the new connected network check.

Proximity should be working now

• Longer names break the scroll bar, although I think this might be a Factorio issue.

This seems to be a Factorio issue yea

[Cooldude2606]

All previous comments have been adressed, I have a few further suggestions for colours and tooltips.

• The tooltips will look better if kepted short and palced onto a new line. I suggest Go to x __1__ y __2__\nDistance Bypass and Go to x __1__ y __2__\nNetwork Bypass.
• On the topic of tooltips, adding \nCooldown Bypass onto the progress bar if the player has the permission would keep styling consistent, this information is not currenrly present anywhere.
• Using grey for the current warp does not provide enough contrast against the white of the other names so I suggest having the current warp show as blue.
• Blue can also be used to show that a player is unable warp because of they cooldown.
• Final style suggestion, using blue for all warps when a player is not on a warp but has bypass does not make much sence to me, this will be the base state of the warps so I think it should be white.
• One coding suggestion I found while playing with the colours, your if statements in update_icon_button can be reduced with the following format:

if not warp_player_is_on then
    if bypass_warp_proximity then
        --[[ Block - Bypass Not In Range ]]
    else
        --[[ Block - Not In Range ]]
    end
elseif warp_player_is_on.warp_id == warp.warp_id then
    --[[ Block - Same Warp ]]
elseif on_cooldown then
    --[[ Block - Cooldown ]]
else
    if warp_electric_network_id == player_warp_electric_network_id then
        --[[ Block - Allowed ]]
    elseif bypass_warp_proximity then
        --[[ Block - Bypass Different Network ]]
    else
        --[[ Block - Different Network ]]
    end
end

The following shows the warp system I used to test and the colours I described above.

[bbassie]

@Cooldude2606 Please fix CI & review my latest commit.

[Cooldude2606]

Git actions have beeen fixed, the fix has been merged into your pr.

[Cooldude2606]

Due to the time that I am reviewing this, I have not tested in game or looked at styling.
Most of the comments are suggestions that could be considered rather than required changes.
I will review the styling tomorrow.

[Cooldude2606]

All suggested changes which where implemented look correct.
I will do a final check in game before I approve the PR.

[Cooldude2606]

Comments from in game testing:

• The "bypass" and "same warp" feel smaller than the other icons, manual size adjustment might be required.
• The "not in range" icon feels too large compered to the other icons, manual size adjustment might be required.
• The description of "bypass" in the sub tooltip is too long, excluding it based on permissions would be a bonus.
• The info doesnt sit right, idk if its because its the only blue thing or if its the size, but something feels off with it. (it could be its new and im not used to seeing it)
• Status icons are not vertically centered, when the name wraps onto a new line the icon does not remained centered.
• Making the gui a bit wider to compensate for the room now taken up by the icon would be a plus. (maybe only when the player has edit accessed, since without edit the name can expand further right)
• Should presses "Discard changes" remove a warp with the edit was opened as a result of presing "New warp"? (i think this would require a bit of work todo and would not provide much benefit beyound consistency)

[bbassie]

Comments from in game testing:

• The "bypass" and "same warp" feel smaller than the other icons, manual size adjustment might be required.
• The "not in range" icon feels too large compered to the other icons, manual size adjustment might be required.

When I have the time I'll look for better icons, doing custom formatting for each icon seems too over done.

• The description of "bypass" in the sub tooltip is too long, excluding it based on permissions would be a bonus.

Will change the description, doing the second part maybe.

• The info doesnt sit right, idk if its because its the only blue thing or if its the size, but something feels off with it. (it could be its new and im not used to seeing it)

Think because it's new, maybe good to add to other GUI's to inform users about what it does.

• Status icons are not vertically centered, when the name wraps onto a new line the icon does not remained centered.
• Making the gui a bit wider to compensate for the room now taken up by the icon would be a plus. (maybe only when the player has edit accessed, since without edit the name can expand further right)

I'll make it single line and increase the with if it gets too small (like you said maybe per user, but we'll see).

• Should presses "Discard changes" remove a warp with the edit was opened as a result of presing "New warp"? (i think this would require a bit of work todo and would not provide much benefit beyound consistency)

Will look into this, it’s interesting!

[Cooldude2606]

• New icons look great, even if this was only a result of updating to 1.1
• Remove warp on first edit works as expected, but it might be possible to use a boolean?
• One change was missed relating to playing a sound, one was corrected and the other not.
• Because of the truncated warp name, could the full name be added at the top of the tooltip.
• I will be asking for an additional review of the locale file to ensure it is consistent.

[mark9064]

I might've missed a few things as always. I also haven't checked any variable names for errors. Looks pretty good overall though!

[Cooldude2606]

My last review covered all functional code, and marks review covered all spelling and grammar, with my last changes which implimented marks request, I am happy to merge this pr.