andymeneely / squib

A Ruby DSL for prototyping card games.
http://squib.rocks
MIT License
918 stars 67 forks source link

Default save_pdf location is incorrect #380

Open wontonst opened 1 year ago

wontonst commented 1 year ago

It's saving as sheet.png instead of sheet.pdf

To Reproduce Steps to reproduce the behavior:

require 'squib'

data = Squib.csv  file: 'equipment.csv'

Squib::Deck.new cards: data['Name'].size, layout: 'equipment.yml' do
  background color: 'green'
  rect
  text str: data['Name'], layout: 'title'
  text str: data['Effects'], layout: 'description'
  text str: data['End game value'], layout: 'lower_right'
  text str: data['Cost'], layout: 'lower_left'
  png file: 'placeholder.png', layout: 'art'
  save_pdf
end

Run on latest dockerized squib

docker run --rm -v "$PWD":/usr/src/app andymeneely/squib ruby main.rb

See that sheet.png is generated. I can rename sheet.png to sheet.pdf and open it fine.

Expected behavior I expect the output file for save_pdf to default to sheet.pdf

Environment M1 mac oxs 13 docker v24

blackwood commented 1 year ago

Can confirm, produces a file called "sheet.png" (which MacOS won't open) on my install too. Overriding with the "file" parameter on save_pdf method fixed it, but it was a little unintuitive after following the docs.

joem commented 6 months ago

I started looking into this a bit when I was bothered by it, and I haven't tried to fix it yet, but I think I traced the problem...

The save_pdf method comes from /lib/squib/dsl/save_pdf.rb which uses /lib/squib/graphics/save_pdf.rb and that uses sheet.full_filename which comes from /lib/squib/args/sheet.rb where it is defined as:

def full_filename(i=nil)
  if i.nil?
    "#{dir}/#{file}"
  else
    "#{dir}/#{prefix}#{count_format % i}#{suffix}.png"
  end
end

At no point is the .png ever overridden. I'm not certain off the top of my head what the best way to fix it would be, but I think a good way that doesn't touch too many files would be making the file extension be one of the properties of sheet, and it defaults to png but can be overridden during the Sheet#full_filename call. I believe that would just require editing /lib/squib/graphics/save_pdf.rb and /lib/squib/args/sheet.rb, and wouldn't affect anything else.

joem commented 6 months ago

Oops. Small update to what I just wrote: it's not actually the full_filename method. Instead I think it's this file parameter in /lib/squib/args/sheet.rb not being overridden: https://github.com/andymeneely/squib/blob/1132233747bae3fc68a6aee6ce27feb132d450c8/lib/squib/args/sheet.rb#L32