Skip to content
This repository has been archived by the owner on Apr 15, 2023. It is now read-only.

How to avoid ambiguous enum values #55

Open
planetis-m opened this issue Dec 22, 2021 · 7 comments
Open

How to avoid ambiguous enum values #55

planetis-m opened this issue Dec 22, 2021 · 7 comments
Labels
enhancement New feature or request

Comments

@planetis-m
Copy link
Collaborator

planetis-m commented Dec 22, 2021

With the last update shortcuts like isKeyDown(Right) broke. We now need to specify the enum type KeyboardKey.Right, this is tedious, however there is a working solution. First we --experimental:overloadableEnums, added in Nim v1.6. .pure needs to be removed from all enums. Last the type of the enum needs to be specified in the proc definition. i.e.: proc isKeyDown*(key: KeyboardKey): bool and proc isMouseButtonDown*(button: MouseButton): bool. Seems to work fine.

@planetis-m planetis-m changed the title How to avoid ambiguos enum values How to avoid ambiguous enum values Dec 22, 2021
@greenfork greenfork added the enhancement New feature or request label Dec 23, 2021
@greenfork
Copy link
Owner

This looks good! Do you have any insight on how stable this experimental feature is? It is at least in the main Nim manual, not the experimental one.

Do you have any idea how exactly pure works and if it is going to be supported in the future? I have seen several threads about how bad it is.

Have you tried just changing the function signature? My understanding is that pure does exactly the thing you have described. So if the function signature is typed, Nim should disambiguate pure enums.

@greenfork
Copy link
Owner

greenfork commented Dec 23, 2021

I found this thread nim-lang/Nim#8066, looks like pure pragma is broken. So it seems as a question which feature is more stable. A good idea would probably be to go for a recently introduced "experimental" solution as it is likely to be better designed. That would also drop support for any Nim earlier than 1.6.

@planetis-m
Copy link
Collaborator Author

planetis-m commented Dec 23, 2021

This looks good! Do you have any insight on how stable this experimental feature is? It is at least in the main Nim manual, not the experimental one.

It's in the experimental manual (although it used to be in the manual before nim-lang/Nim#19173. According to https://forum.nim-lang.org/t/8404#54227 it looks promising. For .pure I think I read, it was going to be removed for v2.

@greenfork
Copy link
Owner

It's in the experimental manual

The link you have referenced is to the normal manual. The experimental manual is https://nim-lang.org/docs/manual_experimental.html

According to https://forum.nim-lang.org/t/8404#54227 it looks promising. For .pure I think I read, it was going to be removed for v2.

I think this sounds good enough to migrate to this feature then. There were some CI issues for MacOS because it failed to install Nim 1.6.0 but hopefully now it is resolved, I will try one more time.

Right now I'm working on #54 where I'm waiting for Windows testing from @enthus1ast and after that I think I will try to incorporate overloadable enums feature.

@planetis-m
Copy link
Collaborator Author

planetis-m commented Dec 23, 2021

Here's a list, I just made of every usage of enums in types and procs. I hope it's complete.

# KeyboardKey
IsKeyPressed
IsKeyDown
IsKeyReleased
IsKeyUp
GetKeyPressed
SetExitKey
SetCameraPanControl
SetCameraAltControl
SetCameraSmoothZoomControl
SetCameraMoveControls
# GamepadButton
IsGamepadButtonPressed
IsGamepadButtonDown
IsGamepadButtonReleased
IsGamepadButtonUp
# GamepadAxis
GetGamepadAxisMovement
# MouseCursor
SetMouseCursor
# MouseButtonPressed
IsMouseButtonPressed
IsMouseButtonDown
IsMouseButtonReleased
IsMouseButtonUp
# Gesture
#SetGesturesEnabled set[Gesture] will not work, see https://forum.nim-lang.org/t/6271#38757 and https://forum.nim-lang.org/t/6383#39375 for the solution
IsGestureDetected
GetGestureDetected
# ConfigFlags
#SetConfigFlags set[ConfigFlags] will not work
IsWindowState
#SetWindowState same as above
ClearWindowState
# TraceLogLevel
TraceLogCallback
SetTraceLogLevel
TraceLog proc type
# CameraProjection
Camera3D.projection
# CameraMode
SetCameraMode
# NPatchLayout
NPatchInfo.layout
# BlendMode
BeginBlendMode
# MaterialMapIndex
SetMaterialTexture
# ShaderUniformDataType
SetShaderValue
SetShaderValueV
# ShaderLocationIndex
Shader.locs # RL_MAX_SHADER_LOCATIONS=32 !
# MaterialMapIndex
Material.maps # MAX_MATERIAL_MAPS=12 !
# PixelFormat
Image.format
Texture.format
LoadImageRaw
ImageFormat
GetPixelColor
SetPixelColor
GetPixelDataSize
# TextureFilter
SetTextureFilter
# TextureWrap
SetTextureWrap
# CubemapLayout
LoadTextureCubemap
# FontType
LoadFontData

@planetis-m
Copy link
Collaborator Author

planetis-m commented Jan 15, 2022

Another idea I got from the forum https://forum.nim-lang.org/t/8810 is to use distinct ints but put KeyboardKey in private/Key.nim , MouseButton in private/MouseButton.nim

Edit: works but doesn't win us anything (except type safety which they all do)
file: lib.nim

import MouseButton, Key
export MouseButton, Key
proc isKeyPressed*(key: KeyboardKey): bool = false

file: app.nim

import gamelib

echo isKeyPressed(Left)
#echo isKeyPressed(Key.Left) #this works

Error:

ambiguous identifier: 'Left' -- use one of the following:
  MouseButton.Left: MouseButton
  Key.Left: KeyboardKey

Pros:

  • shorter names for non conflicting identifiers
  • allows duplicate values

@planetis-m
Copy link
Collaborator Author

This would also work for duplicate values:

type
  MaterialMap = enum
    Diffuse
  ShaderLocationIndex = enum
    Diffuse

template Albedo(_: typedesc[MaterialMap]): untyped = Diffuse
template Albedo(_: typedesc[ShaderLocationIndex]): untyped = Diffuse

proc foo(x: MaterialMap) = discard

foo(MaterialMap.Albedo)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants