search

ikemen-engine / Ikemen-GO

An open-source fighting game engine that supports MUGEN resources.
https://ikemen-engine.github.io
Other
711 stars 125 forks source link

Outdated Windows build instructions #1852

Closed thatvineyard closed 3 months ago

thatvineyard commented 3 months ago

Describe the bug

When following the build instructions there are two issues that arise:

  1. Link to MinGW-w64 download that triggers a trojan warning in Windows Defender
  2. A new version of github.com/qmuntal/gltf/ with breaking changes that cause incompatible types (float32/float64)

To Reproduce

1. MinGW-w64 trojan warning

  1. According to the instructions in the wiki the user is directed to https://winlibs.com/ and to download the MinGW-w64 libraries from there.
  2. Download the latest library
  3. Try to open the zip

2. QMuntal/gltf

  1. Install MinGW and Go as instructed
  2. Navigate to ./build/
  3. Run ./get.cmd
  4. Run ./build.cmd

Expected behavior

1. MinGW-w64 trojan warning Instead of the zip opening, windows defender blocks it and identifies it as a trojan. While it might not be a trojan, it's probably not best to lead users down a path that might require them to circumvent antivirus.

My suggestion is to instead lead them to https://www.msys2.org/, which is the path that Microsoft themselves recommend in their C++ guide. I can suggest the documentation change in the wiki if you want. I have added the suggestion of the modified text below.

2. QMuntal/gltf Expected behavior is that the build will succeed. However the result is the following: 

> .\build.cmd
Building Ikemen GO...

github.com/ikemen-engine/Ikemen-GO/src
# github.com/ikemen-engine/Ikemen-GO/src
src\stage.go:1856:30: cannot use *m.PBRMetallicRoughness.BaseColorFactor (variable of type [4]float64) as [4]float32 value in assignment
src\stage.go:1864:27: cannot use m.AlphaCutoffOrDefault() (value of type float64) as float32 value in assignment
src\stage.go:1878:29: cannot use m.Weights (variable of type []float64) as []float32 value in assignment
src\stage.go:2156:19: cannot use n.Rotation (variable of type [4]float64) as [4]float32 value in assignment
src\stage.go:2157:21: cannot use n.Translation (variable of type [3]float64) as [3]float32 value in assignment
src\stage.go:2158:16: cannot use n.Scale (variable of type [3]float64) as [3]float32 value in assignment
src\stage.go:2161:29: cannot use n.Weights (variable of type []float64) as []float32 value in assignment

I've tracked it down to the gltf library updating their types to float64 in release v0.25.0 and the build script not requiring a version for this library. I have prepared a pull request that fixes this and I have verified that it can run with the Kung Fu Man screenpack: https://github.com/ikemen-engine/Ikemen-GO/pull/1853

I also noted that the github actions use the command go mod download instead of running the ./build/get.cmd script. Which I think is a reason why the builds haven't encountered this issue. It might also be because of the mod cache. But I am not experienced enough with Go's modules to know exactly.

Screenshots / Video

No response

Engine Version (or source code date)

2024-06-24

Operating system

Windows

Extra context or search terms

This is my first foray into Ikemen-GO. I tried reading through CONTRIBUTE.md to make sure I wasn't missing anything, but I apologize if I've done something incorrectly :)

Suggestion for modified wiki text: 

### Part 1: Setting up the compiler

Step 1: Install MinGW-w64 toolchain. Follow the step called "Installing the MinGW-w64 toolchain" in [this guide](https://code.visualstudio.com/docs/cpp/config-mingw#_installing-the-mingww64-toolchain)

-----------------------------------------------------------------------------------------
Step 2: Install Go. The latest version is recommended.
https://golang.org/
SuperFromND commented 3 months ago

Props for the very elegantly written issue!

Thanks for the heads-up on that MinGW thing, I haven't installed it in a while so I wasn't aware of the (likely false-positive) antivirus trigger. I've updated the wiki text to your suggestion 👍

thatvineyard commented 3 months ago

Since get.cmd and build.cmd are outdated files and will removed as a separate issue, this issue can be closed :)